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

tests: refactor to use callTest(s) pattern + move calls to tests/default.nix #2433

Merged
merged 3 commits into from
Oct 18, 2024

Conversation

MattSturgeon
Copy link
Member

@MattSturgeon MattSturgeon commented Oct 18, 2024

  • tests: move test derivations to tests/default.nix
  • tests: use callTest pattern
  • tests: cleanup tests to better use callTest pattern

This takes ideas from #2104 (reducing the scope of that PR) and is required for #2430, as otherwise the auto-formatting leads to an unnecessarily large diff.

This PR contains two key ideas in its refactoring:

  • use the callPackage pattern with a nixvim-specific callTest variant
  • rename tests/default.nix -> tests/main.nix since there are other tests in tests/ not "owned" by default.nix
  • move the builk of the implementation in flake-modules/test.nix to a new tests/default.nix, so that we minimize the implementation details contained within flake-modules and allow tests/ to take responsibility for all its test derivations.

@MattSturgeon MattSturgeon requested a review from a team October 18, 2024 09:54
@MattSturgeon MattSturgeon force-pushed the callTest branch 4 times, most recently from 5cb057d to 4c11b44 Compare October 18, 2024 19:45
Move the previous `default.nix` to `main.nix` so that `default.nix` can
be used for defining the set of all test derivations.

`main.nix` is imported by `default.nix`, but is only responsible for the
tests built from `tests/test-sources/`.
Allows using the `callPackage(s)` pattern on tests.

Rather than using `pkgs.callPackage`, we implement our own variant using
`lib.callPackageWith`.

Our variant (`callTest`) includes additional nixvim-specific stuff
commonly used by our tests.
e.g. Prefer taking `pkgs.*` attrs directly as function arguments.
@@ -0,0 +1,69 @@
# Collects the various test modules in tests/test-sources/ and groups them into a number of test derivations
Copy link
Member Author

@MattSturgeon MattSturgeon Oct 18, 2024

Choose a reason for hiding this comment

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

Is there a better name than main.nix?

  • main.nix could be confusing, as default.nix is the "main" entrypoint to a directory
  • modules.nix could make sense (the file is responsible for making tests from the modules in test-sources), but we already have a modules.nix that tests the wrapper modules...
  • generated.nix could make sense too, but we already have a generated.nix test that tests the ci-generated files are valid

Maybe modules.nix is the way to go, if we renamed the current modules.nix to wrappers.nix? But this is making the diff much larger than it really should be for a minor refactoring PR...

Copy link
Member

Choose a reason for hiding this comment

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

I agree modules.nix would be better. We can do it in another PR though.

@MattSturgeon
Copy link
Member Author

@mergify queue

Copy link
Contributor

mergify bot commented Oct 18, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 036e116

Copy link
Contributor

mergify bot commented Oct 18, 2024

This pull request, with head sha 036e11665f819ebf1dddf493ba212c1ec441eaad, has been successfully merged with fast-forward by Mergify.

This pull request will be automatically closed by GitHub.

As soon as GitHub detects that the sha 036e11665f819ebf1dddf493ba212c1ec441eaad is part of the main branch, it will mark this pull request as merged.

It is possible for this pull request to remain open if this detection does not happen, this usually happens when a force-push is done on this branch callTest, this means GitHub will fail to detect the merge.

@mergify mergify bot merged commit 036e116 into nix-community:main Oct 18, 2024
4 checks passed
@mergify mergify bot temporarily deployed to github-pages October 18, 2024 22:17 Inactive
@MattSturgeon MattSturgeon deleted the callTest branch October 18, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants