Skip to content

Restructure certifier test modules; add unit testing capability #7051

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

Merged
merged 20 commits into from
Apr 28, 2025

Conversation

ana-pantilie
Copy link
Contributor

@ana-pantilie ana-pantilie commented Apr 17, 2025

@ana-pantilie ana-pantilie changed the title Add functions for creating mock trace certifier tests Restructure certifier test modules; add unit testing capability Apr 22, 2025
@ana-pantilie ana-pantilie added the No Changelog Required Add this to skip the Changelog Check label Apr 22, 2025
@ana-pantilie ana-pantilie marked this pull request as ready for review April 22, 2025 11:50
srcTests :: [ String ]
srcTests =
[ "inc"
-- TODO: This is currently failing to certify. This will be fixed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramsay-t is there an issue which tracks this todo?

integrationTests =
testGroup "certifier integration tests"
[
-- testGroup "example serialisation certification"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramsay-t is there an issue which tracks this commented out test?

@ana-pantilie ana-pantilie requested review from ramsay-t and a team April 22, 2025 11:51
Comment on lines 67 to 81
testTrivialSuccess1 :: TestTree
testTrivialSuccess1 =
testSuccess
"Trivial success"
FloatDelay
(mkConstant () (1 :: Integer))
(mkConstant () (1 :: Integer))

testTrivialFailure1 :: TestTree
testTrivialFailure1 =
testFailure
"Trivial failure"
FloatDelay
(mkConstant () (1 :: Integer))
(mkConstant () (2 :: Integer))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ramsay-t It should be very easy to add unit tests now, similar to these two. You provide two terms and the specific optimisation tag.

If you want to test larger traces, that should also be very easy by just constructing the trace manually.

Copy link
Member

@zliu41 zliu41 left a comment

Choose a reason for hiding this comment

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

The test names - "integration", "simplifier" and "unit" - aren't quite self-explanatory or accurate.

  • "Integration" could mean integration with the executable, or integration with the plugin, or others.
  • "Integration" and "Unit" are standard terminology in a testing hierarchy, but "Simplifier" isn't
  • The "integration" tests also run the simplifier, except via the executable.
  • CSE isn't actually part of the simplifier.

Suggested names: "Executable", "Optimizer", "AST".

@ana-pantilie
Copy link
Contributor Author

Good idea @zliu41. Applied your suggestions.

@ana-pantilie ana-pantilie merged commit 1057f6d into master Apr 28, 2025
7 checks passed
@ana-pantilie ana-pantilie deleted the ana/add-negative-certifier-tests branch April 28, 2025 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Changelog Required Add this to skip the Changelog Check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants