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

Enable a few tests #511

Merged
merged 2 commits into from
Mar 12, 2025
Merged

Enable a few tests #511

merged 2 commits into from
Mar 12, 2025

Conversation

Jafaral
Copy link
Member

@Jafaral Jafaral commented Mar 12, 2025

No description provided.

Jafaral added 2 commits March 12, 2025 00:52
Signed-off-by: Jafar Al-Gharaibeh <[email protected]>
Signed-off-by: Jafar Al-Gharaibeh <[email protected]>
@Jafaral Jafaral requested a review from Don-Ward March 12, 2025 15:02
Copy link
Collaborator

@Don-Ward Don-Ward left a comment

Choose a reason for hiding this comment

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

This PR looks ok to me but I have a comment on the tests that are enabled (which were recently committed in e43d66be ). They include the running time as part of the test results. Given we have a system that expects actual test results to be identical to a "Gold Standard" output, how is this not almost guaranteed to fail when run on machines with a different performance to the machine that produced the .std file?

I suggest not enabling lib/stand/uniittest whilst we decide what to do.

@Jafaral
Copy link
Member Author

Jafaral commented Mar 12, 2025

I had the same thought. This is a new testing paradigm that I would like to make use of. I'd like to split this into a new directory and add more tests using the framework. In the short term, I at least wanted to make sure there is some test coverage for it. We already have support for ignoring differences in output, which I used in this case to allow the tests to pass if you noticed. By having it run, we can catch issues with visual inspection of the test output at least, even on the CI side.

@Jafaral
Copy link
Member Author

Jafaral commented Mar 12, 2025

@Don-Ward in summary, the test passed the CI on all platforms as part of this PR. So this is not an issue.

@Don-Ward Don-Ward merged commit e6ac4ea into uniconproject:master Mar 12, 2025
24 checks passed
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