-
Notifications
You must be signed in to change notification settings - Fork 235
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
Moving some slow tests to integration #1442
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
def test_general_hx1d_initializer(self, model): | ||
initializer = HX1DInitializer() | ||
initializer.initialize(model.fs.unit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how this initialization time is going to be affected when we move away from block triangularization in #1436 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point, but I suspect it won't have a huge effect in this case. I believe there is another equivalent test for a different configuration that runs fast enough, so I suspect this one is caused more by stiffness in the model than the property initialization. Still, would be worth waiting for #1436 to check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: after #1436 merged, run time for this test is still ~16 seconds, so I'll leave it in integration for now.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1442 /- ##
==========================================
- Coverage 77.74% 76.35% -1.39%
==========================================
Files 394 394
Lines 64953 64953
Branches 14404 14404
==========================================
- Hits 50497 49598 -899
- Misses 11875 12797 922
Partials 2581 2558 -23 ☔ View full report in Codecov by Sentry. |
Fixes None
Summary/Motivation:
Run times for our non-integration tests have been creeping up again, so this PR moves a lot of slow tests (>10s) to integration. This will likely impact our apparent test coverage, as this is calculated based on the
unit
andcomponent
flags only, but cuts the run time for these by nearly 30% on my local machine.Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: