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

fix: Ensure ODD cleanup if sequencer stops in Examples Python tests #3471

Merged
merged 36 commits into from
Aug 14, 2024

Conversation

andiwand
Copy link
Contributor

@andiwand andiwand commented Aug 2, 2024

Looks like pytest will hold on to all the local variables in case a test fails. This becomes a problem if the ODD was constructed and will be constructed in a consecutive test as only one instance can be in flight 🙄

@andiwand andiwand added this to the next milestone Aug 2, 2024
@github-actions github-actions bot added the Component - Examples Affects the Examples module label Aug 2, 2024
Copy link

github-actions bot commented Aug 2, 2024

📊: Physics performance monitoring for 373743d

Full contents

physmon summary

@andiwand
Copy link
Contributor Author

andiwand commented Aug 2, 2024

Sadly it still crashes

@paulgessinger
Copy link
Member

paulgessinger commented Aug 2, 2024

  1. I think it's just pythons GC that does this, not pytest specifically
  2. I think dd4hep does static initialization that never gets cleaned up.

@andiwand
Copy link
Contributor Author

andiwand commented Aug 2, 2024

  1. I think it's just pythons GC that does this, it purest specifically
  2. I think dd4hep does static initialization that never gets cleaned up.

It seems to only happen when the test fails. If I catch the exception and go on and let the test succeed there is no problem. It somehow seems like a pytest problem to me. Do you see a way to surround the problem @paulgessinger ?

Copy link
Member

@paulgessinger paulgessinger left a comment

Choose a reason for hiding this comment

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

Nice! Might have to revisit how we close file handles at the end of the sequencer run.

Examples/Python/tests/test_examples.py Outdated Show resolved Hide resolved
Examples/Python/tests/test_writer.py Outdated Show resolved Hide resolved
Examples/Python/tests/test_reader.py Outdated Show resolved Hide resolved
@github-actions github-actions bot added the Infrastructure Changes to build tools, continous integration, ... label Aug 7, 2024
@andiwand andiwand marked this pull request as ready for review August 9, 2024 14:49
@andiwand andiwand removed the 🚧 WIP Work-in-progress label Aug 9, 2024
Examples/Python/python/acts/_adapter.py Outdated Show resolved Hide resolved
Examples/Python/tests/conftest.py Outdated Show resolved Hide resolved
@andiwand andiwand marked this pull request as draft August 10, 2024 07:06
@andiwand andiwand marked this pull request as ready for review August 10, 2024 08:38
Copy link

sonarcloud bot commented Aug 14, 2024

@kodiakhq kodiakhq bot merged commit b592b39 into acts-project:main Aug 14, 2024
44 checks passed
@andiwand andiwand deleted the ex-fix-pytest-odd-cleanup branch August 14, 2024 12:47
@paulgessinger paulgessinger modified the milestones: next, v36.1.0 Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ambiguity Resolution Component - Examples Affects the Examples module Infrastructure Changes to build tools, continous integration, ...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants