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

rio: Restrict test execution inside nixosTests for Linux devices #345722

Merged
merged 1 commit into from
Oct 5, 2024

Conversation

otavio
Copy link
Contributor

@otavio otavio commented Oct 1, 2024


Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@otavio otavio requested a review from TornaxO7 October 1, 2024 16:14
@otavio otavio mentioned this pull request Oct 1, 2024
13 tasks
@otavio otavio requested a review from mattpolzin October 1, 2024 16:15
@NickCao
Copy link
Member

NickCao commented Oct 1, 2024

This affects practically every package with a linked nixos test, can we find a solution for them all?

E.g. make nixos tests work on darwin, nothing stop us from running qemu there.

@otavio
Copy link
Contributor Author

otavio commented Oct 1, 2024

This would be for sure the best solution, however I don't have access for the Darwin. So, I believe that someone using this operating system will need to work on that.

Meanwhile, I think that this should not forbid the value of this pull request, as this solves a real problem and other derivations do the same already.

@NickCao
Copy link
Member

NickCao commented Oct 1, 2024

Meanwhile, I think that this should not forbid the value of this pull request, as this solves a real problem and other derivations do the same already.

Could you open a tracking issue (if there isn't one already) and link to it in a comment. So we can just grep and replace all the occurrences of this hack when the underlying issue is fixed.

@NickCao
Copy link
Member

NickCao commented Oct 1, 2024

The PR that introduced this problem: #303597

@otavio
Copy link
Contributor Author

otavio commented Oct 1, 2024

@NickCao Honestly, I don't have too much detail about the Darwin issue, so it would be better if someone with more involvement in Darwin can report that. I'm totally fine to update the code and the commit log to mention this issue so it can be searched in the future.

Don't get me wrong, I'm fine in reporting issues, just I don't have the details to be able to do a proper issue with helpful information.

@mattpolzin Could you help on that?

@NickCao
Copy link
Member

NickCao commented Oct 1, 2024

Also cc @wegank

@mattpolzin
Copy link
Contributor

mattpolzin commented Oct 2, 2024

Sure, I can write up a ticket. I actually didn't realize this was related to only recently starting to run the nixos tests on darwin at all; thanks for the link Nick.

[EDIT] as indicated in the GitHub feed for this PR as well, the ticket I opened can be found here: #345825

Restrict test execution inside nixosTests for Linux devices as ofborg
'passthru.tests' nixosTests are failing on Darwin architectures.

Ref: NixOS#345825
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
@otavio
Copy link
Contributor Author

otavio commented Oct 4, 2024

@mattpolzin I have applied the changes to this pull request. Could you review it?

Copy link
Contributor

@mattpolzin mattpolzin left a comment

Choose a reason for hiding this comment

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

Thanks. We may as well let the aarch64-darwin test play out (though clearly things are being skipped as desired for the already-finished x86_64-darwin passthru tests) before merging.

@mattpolzin mattpolzin merged commit bb1e5d8 into NixOS:master Oct 5, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants