Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows conflict test failures on main #1777

Merged
merged 3 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ jobs:
timeout-minutes: 60
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macOS-latest]
rust: [stable, beta]
Expand Down
13 changes: 12 additions & 1 deletion zebra-test/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ pub struct TestChild<T> {

impl<T> TestChild<T> {
/// Kill the child process.
///
/// ## BUGS
///
/// On Windows, this function can return `Ok` for processes that have
/// panicked. See #1781.
#[spandoc::spandoc]
pub fn kill(&mut self) -> Result<()> {
/// SPANDOC: Killing child process
Expand Down Expand Up @@ -354,7 +359,13 @@ impl<T> TestChild<T> {
.unwrap_or(false)
}

fn is_running(&mut self) -> bool {
/// Is this process currently running?
///
/// ## BUGS
///
/// On Windows, this function can return `true` for processes that have
/// panicked. See #1781.
pub fn is_running(&mut self) -> bool {
matches!(self.child.try_wait(), Ok(None))
}
}
Expand Down
53 changes: 39 additions & 14 deletions zebrad/tests/acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ use zebra_chain::{
};
use zebra_network::constants::PORT_IN_USE_ERROR;
use zebra_state::constants::LOCK_FILE_ERROR;
use zebra_test::{command::TestDirExt, prelude::*};
use zebra_test::{
command::{ContextFrom, TestDirExt},
prelude::*,
};
use zebrad::config::ZebradConfig;

/// The amount of time we wait after launching `zebrad`.
Expand Down Expand Up @@ -1226,24 +1229,46 @@ where
// In node1 we want to check for the success regex
// If there are any errors, we also want to print the node2 output.
let output1 = node1.wait_with_output();
let (output1, node2) = node2.kill_on_error(output1)?;
let res1 = output1.stdout_contains(first_stdout_regex);
let (_, node2) = node2.kill_on_error(res1)?;
let res1 = output1
// This mut is only used on cfg(unix), due to #1781.
#[allow(unused_mut)]
let (output1, mut node2) = node2.kill_on_error(output1)?;

// node2 should have panicked due to a conflict. Kill it here anyway, so it
// doesn't outlive the test on error.
//
// This code doesn't work on Windows. It's cleanup code that only runs when
// node2 doesn't panic as expected. So it's ok to skip it. See #1781.
#[cfg(unix)]
if node2.is_running() {
use color_eyre::eyre::eyre;
return node2
.kill_on_error::<(), _>(Err(eyre!(
"conflicted node2 was still running, but the test expected a panic"
)))
.context_from(&output1)
.map(|_| ());
}
// Now we're sure both nodes are dead, and we have both their outputs
let output2 = node2.wait_with_output().context_from(&output1)?;

// Look for the success regex
output1
.stdout_contains(first_stdout_regex)
.context_from(&output2)?;
use color_eyre::Help;
output1
teor2345 marked this conversation as resolved.
Show resolved Hide resolved
.assert_was_killed()
.wrap_err("Possible port conflict. Are there other acceptance tests running?");
let (_, mut node2) = node2.kill_on_error(res1)?;

// node2 should have panicked due to a conflict. Kill it here
// anyway, so it doesn't outlive the test on error.
let _ = node2.kill();
.warning("Possible port conflict. Are there other acceptance tests running?")
.context_from(&output2)?;

// In the second node we look for the conflict regex
let output2 = node2.wait_with_output()?;
output2.stderr_contains(second_stderr_regex)?;
output2
.stderr_contains(second_stderr_regex)
.context_from(&output1)?;
output2
.assert_was_not_killed()
.wrap_err("Possible port conflict. Are there other acceptance tests running?")?;
.warning("Possible port conflict. Are there other acceptance tests running?")
.context_from(&output1)?;

Ok(())
}