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

Improve COR and tilt validation in test_correlate and test_minimise #2326

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

ashmeigh
Copy link
Collaborator

Issue

Closes #2281

Description

Updated test_correlate and test_minimise to validate COR and tilt before and after actions. Tests now account for cases where COR doesn't change if it's already optimal while ensuring tilt remains constant.

Testing

Verified both tests, ensuring COR changes appropriately and tilt remains unchanged.

Acceptance Criteria

Run the tests and confirm COR and tilt values are validated correctly, with no failures when COR remains unchanged.

@coveralls
Copy link

coveralls commented Sep 10, 2024

Coverage Status

coverage: 74.342%. remained the same
when pulling 72f463f on 2281_test_correlate_check_COR_and_tilt
into e7b6296 on main.

Comment on lines 62 to 63
assert final_cor_value != expected_initial_cor
assert final_tilt_value != expected_initial_tilt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the checks that things are different. Check that the initial values are what is expected, I thin 0 and 64.

Comment on lines 65 to 66
assert final_cor_value == expected_final_cor
assert final_tilt_value == expected_final_tilt
Copy link
Collaborator

Choose a reason for hiding this comment

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

All of the asserts should be self.assertEqual() as that gives better messages.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not why, but the PR is trying to delete this file.

@ashmeigh ashmeigh force-pushed the 2281_test_correlate_check_COR_and_tilt branch from f5a2c81 to 72f463f Compare October 2, 2024 08:20
Copy link
Collaborator

@samtygier-stfc samtygier-stfc left a comment

Choose a reason for hiding this comment

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

Looks good now. Good to have some tests of the values.

I have checked back to release-2.7.0 to see that no recent code changes have modified the values found by these functions.

@samtygier-stfc samtygier-stfc added this pull request to the merge queue Oct 2, 2024
Merged via the queue into main with commit 8664c3a Oct 2, 2024
8 checks passed
@samtygier-stfc samtygier-stfc deleted the 2281_test_correlate_check_COR_and_tilt branch October 2, 2024 16:33
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.

TestGuiSystemReconstruction test_correlate should check COR and tilt
3 participants