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

Unit Test Improvements #5087

Merged
merged 22 commits into from
Jun 23, 2023

Conversation

SchrodingersGat
Copy link
Member

@SchrodingersGat SchrodingersGat commented Jun 22, 2023

  • Select when migration tests are performed
  • Compare how long the unit tests take

Closes #4821

Not running the migration tests shaves 10 minutes (25%) off the unit testing

- Compare how long the unit tests take
@SchrodingersGat SchrodingersGat added the CI CI / unit testing ecosystem label Jun 22, 2023
@SchrodingersGat SchrodingersGat added this to the 0.12.0 milestone Jun 22, 2023
- To get unit tests to run
@matmair
Copy link
Contributor

matmair commented Jun 22, 2023

@SchrodingersGat has slowtests a sizeable impact?

@SchrodingersGat
Copy link
Member Author

@matmair in summary:

  • Migration unit tests are broken out into a separate CI step, allowing them to run in parallel
  • Migration unit tests only run when migrations are touched
  • Migration unit tests take ~20 minutes by themselves
  • Other unit tests have been reduced to ~20 minutes also

Overall this is a 50% reduction in unit testing time - further improvements can still be made but I suspect with diminishing returns.

@SchrodingersGat SchrodingersGat merged commit 3b4e20b into inventree:master Jun 23, 2023
15 checks passed
@SchrodingersGat SchrodingersGat deleted the migration-testing branch June 23, 2023 07:26
@matmair
Copy link
Contributor

matmair commented Jun 23, 2023

Looks great @SchrodingersGat ! Is there any inherent reason to spilt out the migration tests into a separate file? The job could also be run parallel as a separate job in the main qc.yaml IIRC.

@SchrodingersGat
Copy link
Member Author

No inherent reason, I just thought it made sense to keep them distinct. The ci checks file is getting very difficult to understand

@matmair
Copy link
Contributor

matmair commented Jun 24, 2023

Would you be open to merging the files? Having one QC file makes monitoring and automation easier downstream.

@SchrodingersGat
Copy link
Member Author

I'm not opposed to it :) if you submit a PR I'll merge!

matmair added a commit to matmair/InvenTree that referenced this pull request Jun 25, 2023
@matmair matmair mentioned this pull request Jun 25, 2023
SchrodingersGat pushed a commit that referenced this pull request Jun 25, 2023
* merge workflows
from #5087

* syntax fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI / unit testing ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Unit testing for notifications takes excessive amount of time
2 participants