Skip to content

nixosTests.discourse: fix eval#401991

Closed
piotrkwiecinski wants to merge 1 commit intoNixOS:masterfrom
piotrkwiecinski:discourse-run-test
Closed

nixosTests.discourse: fix eval#401991
piotrkwiecinski wants to merge 1 commit intoNixOS:masterfrom
piotrkwiecinski:discourse-run-test

Conversation

@piotrkwiecinski
Copy link
Copy Markdown
Contributor

@piotrkwiecinski piotrkwiecinski commented Apr 26, 2025

Part of #386873

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Apr 26, 2025
drupol
drupol reviewed Apr 26, 2025
View reviewed changes
nixos/tests/discourse.nix Outdated
Copy link
Copy Markdown
Contributor

@drupol drupol Apr 26, 2025

Choose a reason for hiding this comment

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

This shouldn't be required, doesn't it?

@GaetanLepage
Copy link
Copy Markdown
Contributor

GaetanLepage commented Apr 26, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 401991 --package nixosTests.discourse

x86_64-linux

✅ 1 test built:
  • nixosTests.discourse

aarch64-linux

✅ 1 test built:
  • nixosTests.discourse

x86_64-darwin

✅ 1 test built:
  • nixosTests.discourse

aarch64-darwin

✅ 1 test built:
  • nixosTests.discourse

@piotrkwiecinski piotrkwiecinski marked this pull request as draft April 26, 2025 11:57
@piotrkwiecinski
Copy link
Copy Markdown
Contributor Author

piotrkwiecinski commented Apr 26, 2025

Marking as draft as it breaks discourse.passthru.tests:

nixpkgs/pkgs/servers/web-apps/discourse/default.nix

Lines 436 to 440 in 9d47e86

tests = import ../../../../nixos/tests/discourse.nix {
inherit (stdenv) system;
inherit pkgs;
package = pkgs.discourse.override args;
};

@drupol
Copy link
Copy Markdown
Contributor

drupol commented Apr 26, 2025

Marking as draft as it breaks discourse.passthru.tests:

nixpkgs/pkgs/servers/web-apps/discourse/default.nix

Lines 436 to 440 in 9d47e86

tests = import ../../../../nixos/tests/discourse.nix {
inherit (stdenv) system;
inherit pkgs;
package = pkgs.discourse.override args;
};

Yeah, this is why the inherit package; is needed back.

piotrkwiecinski
piotrkwiecinski commented Apr 26, 2025
View reviewed changes
pkgs/servers/web-apps/discourse/default.nix Outdated
Copy link
Copy Markdown
Contributor Author

@piotrkwiecinski piotrkwiecinski Apr 26, 2025

Choose a reason for hiding this comment

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

I'm not sure if this is correct approach.

Copy link
Copy Markdown
Member

@roberth roberth May 28, 2025

Choose a reason for hiding this comment

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

This would be really annoying to get right without #299542

@piotrkwiecinski piotrkwiecinski marked this pull request as ready for review April 28, 2025 13:55
@drupol drupol marked this pull request as draft May 28, 2025 13:10
@roberth roberth mentioned this pull request May 28, 2025
Merged
13 tasks
@mweinelt mweinelt mentioned this pull request May 28, 2025
Merged
13 tasks
@piotrkwiecinski piotrkwiecinski force-pushed the discourse-run-test branch 2 times, most recently from 50c6831 to 77f54bb Compare May 28, 2025 15:59
@piotrkwiecinski piotrkwiecinski changed the title nixosTests.discourse: migrate to runTest nixosTests.discourse: fix eval May 28, 2025
@piotrkwiecinski
Copy link
Copy Markdown
Contributor Author

piotrkwiecinski commented May 28, 2025

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 401991

Logs: https://github.com/piotrkwiecinski/nixpkgs-review-gha/actions/runs/15306585608

x86_64-linux

✅ 1 test built:
  • nixosTests.discourse
✅ 1 package built:
  • discourse.passthru.tests._module.args.package.content

aarch64-linux

✅ 1 test built:
  • nixosTests.discourse
✅ 1 package built:
  • discourse.passthru.tests._module.args.package.content

x86_64-darwin

⏩ 2 packages marked as broken and skipped:
  • discourse.passthru.tests._module.args.package.content
  • nixosTests.discourse

aarch64-darwin

⏩ 2 packages marked as broken and skipped:
  • discourse.passthru.tests._module.args.package.content
  • nixosTests.discourse

@piotrkwiecinski piotrkwiecinski requested a review from mweinelt May 28, 2025 17:45
@piotrkwiecinski piotrkwiecinski marked this pull request as ready for review May 28, 2025 17:45
@piotrkwiecinski
Copy link
Copy Markdown
Contributor Author

piotrkwiecinski commented May 28, 2025

I'll wait for #299542 to attempt fixing darwin

@piotrkwiecinski piotrkwiecinski marked this pull request as draft May 30, 2025 08:39
@Prince213 Prince213 mentioned this pull request May 31, 2025
Closed
13 tasks
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 25, 2025
@mweinelt mweinelt closed this Aug 4, 2025
@piotrkwiecinski piotrkwiecinski deleted the discourse-run-test branch January 27, 2026 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@drupol drupol drupol left review comments

@roberth roberth roberth left review comments

@GaetanLepage GaetanLepage Awaiting requested review from GaetanLepage

@mweinelt mweinelt Awaiting requested review from mweinelt

+1 more reviewer

@a-kenji a-kenji a-kenji approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@piotrkwiecinski @GaetanLepage @drupol @roberth @a-kenji @mweinelt