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 help test in dev shell #11900

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

edolstra
Copy link
Member

Motivation

Not sure what the intent was expecting help.sh to fail in the main suite, but it caused meson test to fail inside a nix develop shell:

$ meson test help
ninja: Entering directory `/home/eelco/Dev/nix-master/build'
1/1 nix-functional-tests:main / help        UNEXPECTEDPASS   4.02s

Probably the condition should be something like

      should_fail : script == 'help.sh' and (not get_option('doc-gen') or meson.is_cross_build())

but we can't access the doc-gen option in tests/functional/meson.build....

Context

Priorities and Process

Add 👍 to pull requests you find important.

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

Not sure what the intent was expecting help.sh to fail in the main suite, but it caused `meson test` to fail inside a `nix develop` shell:

  $ meson test help --print-errorlogs
  ninja: Entering directory `/home/eelco/Dev/nix-master/build'
  1/1 nix-functional-tests:main / help        UNEXPECTEDPASS   4.02s
@edolstra edolstra requested a review from Ericson2314 November 18, 2024 15:23
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Nov 18, 2024
@@ -167,7 +167,7 @@ suites = [
'impure-env.sh',
'debugger.sh',
'extra-sandbox-profile.sh',
'help.sh',
#'help.sh',
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to exclude it unconditionally.

Suggested change
#'help.sh',
'help.sh',

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately one of the VM tests fails without this, see https://github.com/NixOS/nix/actions/runs/11895552419/job/33145394782.

Copy link
Member

Choose a reason for hiding this comment

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

Discussed in meeting
Possible solutions

  • add nix-manual.man manpages to NixOS test common.nix <= preferred
  • add nix (everything.nix) preferably not because that includes the functional tests
  • add a meson option to disable help.sh

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-12-16-nix-team-meeting-minutes-203/57483/1

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