-
Notifications
You must be signed in to change notification settings - Fork 59
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
GAC Test Harness #1311
GAC Test Harness #1311
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1311 +/- ##
=======================================
Coverage 94.39% 94.39%
=======================================
Files 371 371
Lines 37922 37922
=======================================
Hits 35796 35796
Misses 2126 2126 ☔ View full report in Codecov by Sentry. |
I think the best thing to do is try to get this merged as the GAC unit test harness PR, and then have subsequent PR's for the costing test as we decide what we're doing with them. |
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 - I agree that costing tests can be expanded upon later in the watertap.costing.unit_models.tests
directory. My only comment is that I'm unsure if the test for reporting is necessary in the unit model test - although it can't hurt.
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. I agree that its best to leave the costing for another PR.
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.
We should merge #1317 first as it has needed API changes for the UnitTestHarness
.
def configure(self): | ||
m = build_hand() | ||
|
||
self.unit_model_block = m.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.
This is now optional, but it doesn't hurt.
Co-authored-by: bknueven <[email protected]>
Fixes/Resolves:
GAC in #1302
Summary/Motivation:
Consolidating GAC testing using new
UnitTestHarness
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: