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: testing suite (random order) #3519

Merged
merged 71 commits into from
Nov 25, 2024
Merged

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Oct 25, 2024

Description

Implementing random order in the testing #GoodPractice.

Additionally, several more issues and changes spawned from this, like:

Issue linked

Close #3268

Checklist

@germa89 germa89 requested a review from a team as a code owner October 25, 2024 12:54
@germa89 germa89 requested review from clatapie and pyansys-ci-bot and removed request for a team October 25, 2024 12:54
@germa89 germa89 self-assigned this Oct 25, 2024
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added CI/CD Related with CICD, Github Actions, etc dependencies maintenance General maintenance of the repo (libraries, cicd, etc) labels Oct 25, 2024
@germa89 germa89 force-pushed the ci/doing-random-tests-on-ci branch from 0a890bb to e206260 Compare October 25, 2024 15:21
@SMoraisAnsys
Copy link
Contributor

@germa89 While passing by... Would it make sense for you to use pytest-xdist ? This way you could speed up your tests by running them in parallel. If there is a limitation such that one worker has to work on a whole file, you can also use the option --dist loadfile so that each worker is provided with a test file and handling it totally.

@germa89
Copy link
Collaborator Author

germa89 commented Nov 18, 2024

hi @SMoraisAnsys

Thank you a lot for the suggestion! This is a good idea, and believe we did try it in the pass. The main limitation is that I will have to spin several MAPDL instances which is "heavy" since the docker image is huge. I think that is why we did not implement it.

Additionally, so far, our main bottleneck is on the documentation generation where I do want to implement some parallelization technique, probably having more than one MAPDL image.

I appreciate a lot you coming around PyMAPDL repo! :)

Copy link
Contributor

Hello! 👋

Your PR is changing the image cache. So I am attaching the new image cache in a new commit.

This commit does not re-run the CICD workflows (since no changes are made in the codebase) therefore you will see the actions showing in their status Expected — Waiting for status to be reported. Do not worry. You commit workflow is still running here 😄

You might want to rerun the test to make sure that everything is passing. You can retrigger the CICD sending an empty commit git commit -m "chore: empty comment to trigger CICD" --allow-empty.

You will see this message everytime your commit changes the image cache but you are not attaching the updated cache. 🤓

@germa89
Copy link
Collaborator Author

germa89 commented Nov 21, 2024

I knew that changing the order will increase the time... but yisus.... that's a lot. And I think it is just the respawming of the MAPDL what is taking the time.... but I do not have any probe of this.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@germa89 germa89 changed the title ci: using random order when doing the tests refactor: testing suite (random order) Nov 22, 2024
@github-actions github-actions bot added the enhancement Improve any current implemented feature label Nov 22, 2024
@germa89
Copy link
Collaborator Author

germa89 commented Nov 25, 2024

@pyansys-ci-bot LGTM.

@germa89 germa89 enabled auto-merge (squash) November 25, 2024 11:38
Copy link
Contributor

@pyansys-ci-bot pyansys-ci-bot left a comment

Choose a reason for hiding this comment

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

✅ Approving this PR because germa89 said so in here 😬

LGTM

@germa89 germa89 merged commit 3e4aca6 into main Nov 25, 2024
54 of 58 checks passed
@germa89 germa89 deleted the ci/doing-random-tests-on-ci branch November 25, 2024 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Related with CICD, Github Actions, etc dependencies documentation Documentation related (improving, adding, etc) enhancement Improve any current implemented feature maintenance General maintenance of the repo (libraries, cicd, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

testing: random test ordering
3 participants