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

Python test test_set_aperture failing #1401

Closed
fabcor-maxiv opened this issue Sep 18, 2024 · 4 comments · Fixed by #1402
Closed

Python test test_set_aperture failing #1401

fabcor-maxiv opened this issue Sep 18, 2024 · 4 comments · Fixed by #1402
Assignees

Comments

@fabcor-maxiv
Copy link
Contributor

As pointed out in these:

it seems like test_set_aperture is failing... sometimes.

@fabcor-maxiv
Copy link
Contributor Author

According to @elmjag, this might be related to a timing issue, so the test passes sometimes and the other times it fails. If I understood correctly he made an attempt at a fix in this commit elmjag@9e65579.

@beteva
Copy link
Member

beteva commented Sep 18, 2024

The problem comes from the evaluation of the size of the slits. If the slits are smaller than the aperture, than the beam size is taken from the slits and the test_set_aperture fails systematically with 'A100' == 'slits'. The problem is intermittent, because the test are using random aperture size, so when A100 is not used, the tests pass.
There are three solutions in order the tst to pass. Either:

  1. Remove the slits from the beam.xml
    or
  2. Set the slits bigger (or at least equal) of the biggest aperture (A100). The values are hardcoded in the SlitsMockup, so we need to change the mockup to take into account the values, set in the configuration.
    or
  3. Remove the biggest aperture from the tests.

While waiting for the SlitMockup to be changed and to make it take the values from the configuration file, I vote for 1.

@elmjag
Copy link
Contributor

elmjag commented Sep 18, 2024

The problem comes from the evaluation of the size of the slits. If the slits are smaller than the aperture, than the beam size is taken from the slits and the test_set_aperture fails systematically with 'A100' == 'slits'. The problem is intermittent, because the test are using random aperture size, so when A100 is not used, the tests pass. There are three solutions in order the tst to pass. Either:

Oh yes, now I see it. The problem is the fact we pick new aperture randomly.

I suggest that, at least for now, we just always pick the first aperture from the available list. It works and it is still a good test on the diffractometer/aperture routes.

https://github.com/mxcube/mxcubeweb/pull/1402/files#diff-bb6c0006c58e00ff0c309c3cb6fa2a0b64a2aacf108701b8204e583f13f6b1d3R108

In general I think using randomness in these kind of tests is problematic. If we want to test switching to different aperture sizes, we should make a parameterized test instead. We want to keep our test suite as deterministic as possible.

@beteva
Copy link
Member

beteva commented Sep 18, 2024

I suggest that, at least for now, we just always pick the first aperture from the available list. It works and it is still a good test on the diffractometer/aperture routes.

Very good, as the ApertureMockup is also deterministic - A5 is the firs value ans A10 is the one set in the init.

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 a pull request may close this issue.

3 participants