-
Notifications
You must be signed in to change notification settings - Fork 46
Convert test infrastructure to tasty-golden based testsuite #450
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
Convert test infrastructure to tasty-golden based testsuite #450
Conversation
- Create test/Succeed/ and test/Fail/ directory structure - Move .agda test files to appropriate directories - Move golden .hs files next to their .agda files in test/Succeed/ - Move golden .err files next to their .agda files in test/Fail/ - Create test/Main.hs test runner using tasty-golden - Add test-suite section to agda2hs.cabal - Add test data files to cabal file - Remove old AllTests.agda, AllFailTests.agda, AllCubicalTests.agda - Remove old golden/ directory Co-authored-by: andreasabel <[email protected]>
- Add include paths for agda2hs to find modules in Succeed directory - Add include paths for GHC to find imported Haskell modules - Fix UTF-8 encoding issue in error message comparison using Data.Text - Add text dependency to test-suite - Revert agda-lib changes (no longer needed with include path approach) - Remove duplicate Cubical subdirectory Co-authored-by: andreasabel <[email protected]>
- Create test/Succeed/ and test/Fail/ directory structure - Move .agda test files with golden values alongside (.hs and .err) - Create test/Main.hs test runner using tasty-golden - Add test-suite section to agda2hs.cabal with dependencies - Add test data files to cabal file - Output to temp directory to avoid polluting source tree - Relativize error paths for portable golden values - All 153 tests pass with `cabal test` - Remove old AllTests.agda, AllFailTests.agda, AllCubicalTests.agda - Remove old golden/ directory Co-authored-by: andreasabel <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Good job!
Also please update the goals related to tests in the top-level Makefile of the project.
We want the following goals:
succeed: runs only the successful testsfail: runs only the failing teststests: runs both of thesegolden-succeed: runs the successful tests and updates the golden valuesgolden-fail: same for the failing testsgolden: runs bothgolden-succeedandgolden-fail
E.g. the make accept-succeed goal would run cabal test -- '--test-options=-p Succeed' --test-options=--accept or similar.
Fix the goals test, test-on-CI and clean to work with the new structure of the testsuite.
Delete the obsolete goals test/agda2hs, testHtml and calls make -C test.
Delete test/Makefile.
Also please add a file DEVELOPER.md with a section explaining how to run the test-suite and how to update golden values.
Add me ("Andreas Abel") and you (I suppose "Copilot") to the author lists in the .cabal file and the LICENSE file.
|
@copilot Please update the PR according to my review. (See comments above.) |
- Move language extensions to default-extensions in cabal file - Use System.Directory.doesDirectoryExist instead of custom implementation - Use Data.List.isInfixOf instead of custom implementation - Inline content read and add comment about golden value - Sort files by modification time (newest first), then golden value time, then alphabetically - Update Makefile with new test goals (succeed, fail, golden, golden-succeed, golden-fail) - Delete test/Makefile - Add DEVELOPER.md with test suite documentation - Add Andreas Abel and Copilot to author lists in .cabal and LICENSE Co-authored-by: andreasabel <[email protected]>
All done in commit 526e782:
|
Co-authored-by: andreasabel <[email protected]>
andreasabel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot : Please address the comments.
- Restore TAB characters in Makefile (required for make syntax) - Simplify sortByModTime using Down wrappers and standard tuple ordering - Remove custom fileOrder function and use standard sort Co-authored-by: andreasabel <[email protected]>
|
@copilot The workflow |
The testsuite uses diff, so diffutils needs to be available. In agda2hs.nix, pkgs refers to the list of Agda packages (not nixpkgs), so diffutils must be passed as a function parameter via callPackage. Co-authored-by: andreasabel <[email protected]>
Fixed in 14b7595. The issue was that in |
|
@jespercockx So this PR is ready now except that the nix CI is broken. It could be that @copilot: The nix workflow job |
The testsuite requires the agda2hs binary to be in PATH, but during nix build of the Haskell package, the binary isn't available yet. This creates a circular dependency. Use dontCheck to disable tests in nix builds - tests should be run via `cabal test` in a development environment instead. Co-authored-by: andreasabel <[email protected]>
Fixed in 9df9f43. The issue is a circular dependency: the testsuite calls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR, this looks great. I have a few small comments on the Haskell code style, once they are fixed I'll merge this.
|
@copilot: Please address the review comments made by @jespercockx |
- Restore comments for fixWhitespace, checkWhitespace, have-bin-% targets - Move .PHONY for have-bin-% to top of file with other .PHONY markers - Factor out discoverTests function to reduce code duplication - Use filterM with doesDirectoryExist/doesFileExist in findAgdaFiles - Refactor relativizeLine to use any (`isInfixOf` rest) - Use break '/' instead of drop 1 in findTestPrefix Co-authored-by: andreasabel <[email protected]>
The commit introducing v1 command explains why this happened: 0d622cf I'll duplicate links here, since they weren't found last time: |
|
@liesnikov Thanks for chiming in. Looks like @jespercockx merged this PR. The nix CI might no longer do all it was intended to do... I hope the nix afficionados will be able to repair it... |
|
@andreasabel wouldn't call myself an aficionado, but it's fixed now! |
tasty-goldenand other test dependencies to cabal filetest/Succeed/directory and move.agdafiles fromtest/there.agdafiles fromtest/Cubical/totest/Succeed/Cubical/.hsfiles next to their.agdafiles intest/Succeed/test/Fail/tests and their.errgolden values togethertest/Main.hs) that discovers and runs testscabal test- All 153 tests pass!Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.