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

Refactor tests #128

Merged
merged 44 commits into from
Dec 3, 2024
Merged

Refactor tests #128

merged 44 commits into from
Dec 3, 2024

Conversation

y4izus
Copy link
Collaborator

@y4izus y4izus commented Nov 15, 2024

Details and comments

Done in this PR:

  • Unify all the synthesis passes tests (different types and local+cloud mode) on one file. Since local synthesis for Pauli doesn't exist yet, there is a skip for that case
  • Unify all the collector passes tests (different types and local+cloud mode) on one file
  • Unify local and cloud routing pass tests on one file
  • Rename passes files for easy reading
  • Group all tests on the same folder
  • Unify conftest files
  • Group all the fixtures on the conftest file
  • Create parametrize functions to not repeat parametrize configs
  • Create a file for parametrize functions that are common to more than one file.
  • Add several TODO and FIXME comments for things that I think we should review
  • Fix some errors on the code found when refactoring the tests

More posible refactor where I would like to hear your thoughts:

  • We could move all the parametrize functions to the same file the same way we have with fixtures, so we can see easily what is done and if we can reuse it. Also, we could even create structures with their values so, in the case of adding a new tests we just had to change one place. Example: most of the parametrizations has the Collector, so we could create a dict with that info and access to the dict on the parametrize function.
  • We could create a new file for utils. Right now we have 2 functions defined to create circuits for the tests (plus other functions imported). We could create that file to have that creations together. Although I think we could first review the functions just in case we are creating several functions for doing the same
  • Some TranspilerService tests are similar to other passes tests like: wrong token, wrong url, exceed time, .... Should I remove that tests from the passes?

Thanks!

@y4izus y4izus self-assigned this Nov 20, 2024
@y4izus y4izus marked this pull request as ready for review November 20, 2024 12:02
Copy link
Collaborator

@victor-villar victor-villar left a comment

Choose a reason for hiding this comment

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

Nice work!
I thin we need to fix the circuits, either by setting the seed in all random generation steps or by creating/passing fixed circuits. Otherwise the pipeline will fail randomly.
We can fix other todos later.

tests/test_ai_synthesis_pass.py Outdated Show resolved Hide resolved
tests/test_ai_synthesis_pass.py Outdated Show resolved Hide resolved
tests/test_transpiler_service.py Outdated Show resolved Hide resolved
tests/test_transpiler_service.py Outdated Show resolved Hide resolved
@y4izus y4izus merged commit c07eabd into main Dec 3, 2024
4 checks passed
@y4izus y4izus deleted the yg-refactor-tests branch December 3, 2024 15:39
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.

4 participants