Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a package type test to verify that parametric owned modules don't create duplicate package definitions, and refactors the test assertions into individual checks.
- Added a
pkgoption of typepackageto test that packages cannot be duplicated in parametric owned modules - Modified the
nixosconfiguration to be a function acceptingpkgsto enable package assignment - Split the single combined test into separate individual test checks for better granularity
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| checks.parametric-fwd-a = checkCond "fwd-a" (rockhopper.config.fwd.a == "First owned A"); | ||
| checks.parametric-fwd-b = checkCond "fwd-b" ( | ||
| rockhopper.config.fwd.b == "Second owned B for rockhopper" | ||
| ); | ||
| checks.parametric-fwd-c = checkCond "fwd-c" (rockhopper.config.fwd.c == "host owned C"); | ||
| checks.parametric-fwd-d = checkCond "fwd-d" (rockhopper.config.fwd.d == "First static includes D"); | ||
| checks.parametric-fwd-e = checkCond "fwd-e" (rockhopper.config.fwd.e == "Third Impact"); | ||
| checks.parametric-fwd-f = checkCond "fwd-f" (rockhopper.config.fwd.f == "Fifth Earth rockhopper"); | ||
| checks.parametric-fwd-pkg = checkCond "fwd-pkg" (lib.getName rockhopper.config.fwd.pkg == "hello"); |
There was a problem hiding this comment.
[nitpick] The test name descriptions ("fwd-a", "fwd-b", etc.) are less descriptive than the pattern used in other test files in this codebase. Other tests use descriptive messages like "home-managed fish for alice", "den.default.user.includes defines user on host", or "added hello at user packages" that explain what behavior is being tested.
Consider using more descriptive names like:
- "fwd-a" → "First owned A forwards correctly"
- "fwd-b" → "Second owned B forwards with host name"
- "fwd-c" → "host owned C forwards correctly"
- "fwd-d" → "First static includes D forwards correctly"
- "fwd-e" → "Third Impact forwards correctly"
- "fwd-f" → "Fifth Earth forwards with planet and host"
- "fwd-pkg" → "package forwards without duplication"
| checks.parametric-fwd-a = checkCond "fwd-a" (rockhopper.config.fwd.a == "First owned A"); | |
| checks.parametric-fwd-b = checkCond "fwd-b" ( | |
| rockhopper.config.fwd.b == "Second owned B for rockhopper" | |
| ); | |
| checks.parametric-fwd-c = checkCond "fwd-c" (rockhopper.config.fwd.c == "host owned C"); | |
| checks.parametric-fwd-d = checkCond "fwd-d" (rockhopper.config.fwd.d == "First static includes D"); | |
| checks.parametric-fwd-e = checkCond "fwd-e" (rockhopper.config.fwd.e == "Third Impact"); | |
| checks.parametric-fwd-f = checkCond "fwd-f" (rockhopper.config.fwd.f == "Fifth Earth rockhopper"); | |
| checks.parametric-fwd-pkg = checkCond "fwd-pkg" (lib.getName rockhopper.config.fwd.pkg == "hello"); | |
| checks.parametric-fwd-a = checkCond "First owned A forwards correctly" (rockhopper.config.fwd.a == "First owned A"); | |
| checks.parametric-fwd-b = checkCond "Second owned B forwards with host name" ( | |
| rockhopper.config.fwd.b == "Second owned B for rockhopper" | |
| ); | |
| checks.parametric-fwd-c = checkCond "host owned C forwards correctly" (rockhopper.config.fwd.c == "host owned C"); | |
| checks.parametric-fwd-d = checkCond "First static includes D forwards correctly" (rockhopper.config.fwd.d == "First static includes D"); | |
| checks.parametric-fwd-e = checkCond "Third Impact forwards correctly" (rockhopper.config.fwd.e == "Third Impact"); | |
| checks.parametric-fwd-f = checkCond "Fifth Earth forwards with planet and host" (rockhopper.config.fwd.f == "Fifth Earth rockhopper"); | |
| checks.parametric-fwd-pkg = checkCond "package forwards without duplication" (lib.getName rockhopper.config.fwd.pkg == "hello"); |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #101