Skip to content

Conversation

@vic
Copy link
Owner

@vic vic commented Dec 16, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 16, 2025 17:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the test suite by splitting a monolithic test file into individual test modules, improving test organization and maintainability. The changes reorganize the type system definitions and create separate test files for each test case.

  • Simplified type definitions in nix/types.nix by reusing the aspectsType for the provides option
  • Split large test file into 14 individual test modules for better organization
  • Created a new checkmate/ directory structure with dedicated flake and configuration

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
nix/types.nix Simplified type definitions by reusing aspectsType and reordering type union
checkmate/modules/tests/without_flakes.nix Extracted test for usage without flakes into separate module
checkmate/modules/tests/transpose_swap.nix Extracted transpose swap test into separate module
checkmate/modules/tests/transpose_common.nix Extracted transpose common childs test into separate module
checkmate/modules/tests/tranpose_flake_modules.nix Extracted flake modules transpose test into separate module
checkmate/modules/tests/default_empty.nix Extracted default empty test into separate module
checkmate/modules/tests/aspect_toplevel_parametric.nix Extracted top-level parametric aspect test into separate module
checkmate/modules/tests/aspect_provides.nix Extracted aspect provides test into separate module
checkmate/modules/tests/aspect_parametric.nix Extracted parametric aspect test into separate module
checkmate/modules/tests/aspect_modules_resolved.nix Extracted modules resolved test into separate module
checkmate/modules/tests/aspect_fixpoint.nix Extracted fixpoint test into separate module
checkmate/modules/tests/aspect_dependencies.nix Extracted aspect dependencies test into separate module
checkmate/modules/tests/aspect_default_provider_override.nix Extracted default provider override test into separate module
checkmate/modules/tests/aspect_default_provider_functor.nix Extracted default provider functor test into separate module
checkmate/modules/tests/aspect_chain.nix Extracted aspect chain test into separate module
checkmate/modules/tests.nix Refactored to provide shared test utilities and imports
checkmate/modules/checkmate.nix New configuration module for checkmate test infrastructure
checkmate/flake.nix New flake configuration for checkmate test suite
.github/workflows/test.yml Updated workflow to checkout code and run tests from local directory

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,29 @@
{
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'tranpose' to 'transpose' in filename.

Copilot uses AI. Check for mistakes.
{ transpose, ... }:
{

flake.tests."test transpose common childs become one parent" = {
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'childs' to 'children'.

Suggested change
flake.tests."test transpose common childs become one parent" = {
flake.tests."test transpose common children become one parent" = {

Copilot uses AI. Check for mistakes.
three-and-four-and-five = {
classOne.bar = [ "3" ];
includes = [
aspects.four
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

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

The test references aspects.four from the inner fixpoint context, but four is defined in the outer aspects.aspectTwo.provides scope. This should use top.aspects.aspectTwo.provides.four to match the actual scope, or the test may not validate the intended fixpoint behavior correctly.

Suggested change
aspects.four
top.aspects.aspectTwo.provides.four

Copilot uses AI. Check for mistakes.
@vic vic force-pushed the split-tests branch 3 times, most recently from 2dc3431 to be86657 Compare December 16, 2025 17:45
@vic vic merged commit 72ffc00 into main Dec 16, 2025
1 check passed
@vic vic deleted the split-tests branch December 16, 2025 17:49
vic added a commit that referenced this pull request Dec 17, 2025
This reverts commit 72ffc00.
@vic vic mentioned this pull request Dec 17, 2025
vic added a commit that referenced this pull request Dec 17, 2025
vic added a commit that referenced this pull request Dec 17, 2025
vic added a commit that referenced this pull request Dec 17, 2025
This reverts commit 72ffc00.
vic added a commit that referenced this pull request Dec 17, 2025
This reverts commit 72ffc00.
vic added a commit that referenced this pull request Dec 18, 2025
vic added a commit that referenced this pull request Dec 18, 2025
* Revert "Split tests (#19)"

This reverts commit 72ffc00.

* update README with correct test commands (#22)

* Revert "Revert "Split tests (#19)""

This reverts commit 1df56b4.

* test merging aspects

* fix: use custom merge functions for modules and resolve options (#23)

When merging aspect configs across scopes (e.g. bar = config.foo), the
modules and resolve options were incorrectly merging pre-computed values
from individual scopes instead of recomputing from the merged config.

The fix uses custom type merge functions that always recompute from the
final merged config, ensuring resolved modules reflect all merged aspect
definitions.

---------

Co-authored-by: Albert O'Shea <albertoshea2@gmail.com>
vic added a commit that referenced this pull request Dec 18, 2025
vic added a commit that referenced this pull request Dec 18, 2025
vic added a commit that referenced this pull request Dec 18, 2025
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