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

contrib: add direct tests for agenix #163

Merged
merged 1 commit into from
Dec 20, 2023
Merged

contrib: add direct tests for agenix #163

merged 1 commit into from
Dec 20, 2023

Conversation

ryantm
Copy link
Owner

@ryantm ryantm commented Feb 22, 2023

These tests are MUCH faster than the NixOS tests.

@ryantm ryantm force-pushed the rtm-2-21-recursive-nix branch 5 times, most recently from 1e73b48 to 4b71dcc Compare February 26, 2023 17:46
@ryantm ryantm changed the title wip: see if recursive nix works in ci contrib: add direct tests for agenix Feb 26, 2023
@ryantm ryantm marked this pull request as ready for review February 26, 2023 17:46
@ryantm ryantm requested a review from n8henrie February 26, 2023 17:46
Copy link
Collaborator

@n8henrie n8henrie left a comment

Choose a reason for hiding this comment

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

Looking through, it looks like you've migrated the tests to an installcheck on the derivation, which means they would run when the CLI was instantiated (?) / built / better word. Would we then remove the nix flake check process and just install the binary in CI -- if it installs, that means the tests passed?

EDIT: Premature submission. More coming.

Copy link
Collaborator

@n8henrie n8henrie left a comment

Choose a reason for hiding this comment

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

Compared to the existing approach, I think this is fine. Speeding things up test cycles is nice for local development, though I don't much care if things take a minute in CI. Potential downsides include that running in a VM ensures a reproducible environment, both locally and in CI, whereas it looks like these may (?) be impure -- please help me learn if I'm wrong here.

Compared to #159, there are a few things I like about the python module approach:

  • importing from a python module lets me use a whole host of tools that I'm accustomed to when writing tests -- (limited) type checking, syntax highlighting, autocomplete
  • while it will still depend on mutable state, using a python module should make is easier to break out individual tests and make them more functional, if desired -- for example isolating a failure to a specific test in a live environment would be just tester = AgenixTester(vars); tester.run_specific_test()
  • I presume that if we ever wanted to split out tests into separate checks checks.${system}.rekeying_words, the python module would make this easier. Might not be much desired to ever do this though!
  • I believe we could add some doctests to the tests in the python module, making a simple way to "test the test", if desired.

All that said -- the python module approach does add some complexity, take longer, and many of the above points are admittedly premature optimization! It looks like either approach should make it easier and more reliable to test the CLI as $USER, which I think is the first priority.

If the python module approach feels too far from the beaten path, too complex / brittle, or it's just not your preference, I won't take offense!

pkgs/agenix.nix Show resolved Hide resolved
pkgs/agenix.nix Outdated Show resolved Hide resolved
@n8henrie
Copy link
Collaborator

Also, I meant to ask -- other than speed and more directly testing user installs, are there other benefits that you envision from testing this way that I might have missed?

@ryantm
Copy link
Owner Author

ryantm commented Mar 1, 2023

Mainly I'm concerned about the speed of the NixOS tests.

This typically runs in the nix build sandbox, so it doesn't end up messing with the user's environment.

Another totally different path we could go is to merge our efforts with https://github.com/yaxitech/ragenix and stop coding bash, and use Rust's testing tools.

@n8henrie
Copy link
Collaborator

n8henrie commented Mar 3, 2023

Another totally different path we could go is to merge our efforts with https://github.com/yaxitech/ragenix and stop coding bash, and use Rust's testing tools.

I'm a big rust fanboy (though not terribly good at it yet, it's probably what I'm most focusing on learning at this point).

What about #23 ?

@ryantm
Copy link
Owner Author

ryantm commented Mar 4, 2023

My concern with #23 is that I don't want to force everyone to change their secrets.nix file to a TOML file.

These tests are MUCH faster than the NixOS tests.
@ryantm ryantm added the dev label Dec 20, 2023
@ryantm ryantm merged commit 17090d1 into main Dec 20, 2023
6 checks passed
@ryantm ryantm deleted the rtm-2-21-recursive-nix branch December 20, 2023 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants