Skip to content

Improve the timeouts test#14802

Merged
Ericson2314 merged 1 commit intomasterfrom
improve-timeouts-test
Jan 5, 2026
Merged

Improve the timeouts test#14802
Ericson2314 merged 1 commit intomasterfrom
improve-timeouts-test

Conversation

@Ericson2314
Copy link
Member

Motivation

  • More concise
  • Also checks error messages
  • Checks more error codes

Context

The nature of that bug is that if the first command's exit status is correctly 101 and not 1, the rest should be correctly 101, 100, etc. too.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

- More concise
- Also checks error messages
- Checks more error codes

The nature of that bug is that if the first command's exit status is
correctly 101 and not 1, the rest should be correctly 101, 100, etc.
too.
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Dec 15, 2025
fi
# FIXME: https://github.com/NixOS/nix/issues/4813
expectStderr 101 nix-build -Q timeout.nix -A infiniteLoop --timeout 2 | grepQuiet "timed out" \
|| skipTest "Do not block CI until fixed"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skipTest cancels the whole test file, which is unexpected. Easy to miss when reading this test.

|| is for flaky tests, at best. What we should have here is a negation, so that when the behavior is fixed, the test must be updated to reflect the new state and actually test it.

Copy link
Member Author

@Ericson2314 Ericson2314 Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it worked before too, fwiw. Not ideal, but not worse in this case either.

The real condition is that the return code should either by 1xy (good) or 1 (bug). But of course the best thing is just fixing the bug.

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ericson2314 Ericson2314 added this pull request to the merge queue Jan 5, 2026
Merged via the queue into master with commit 6c884ff Jan 5, 2026
20 checks passed
@Ericson2314 Ericson2314 deleted the improve-timeouts-test branch January 5, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants