Skip to content

Add minimal pytest plugin to verify that generated files are cleaned up by tests #1488

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.

Sign up for GitHub

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

Jump to bottom
Merged
lbianchi-lbl merged 9 commits into from
Sep 27, 2024
Merged

Add minimal pytest plugin to verify that generated files are cleaned up by tests #1488

lbianchi-lbl merged 9 commits into from
Sep 27, 2024

Conversation

@lbianchi-lbl
Copy link
Contributor

@lbianchi-lbl lbianchi-lbl commented Sep 11, 2024

Summary/Motivation:

  • @andrewlee94 discovered that a conspicuous number of untracked files are generated when running the test suite
  • I was able to confirm his intuition that this was most likely a regression introduced by Remove idaes.core.dmf and related code #1484
  • It's not entirely clear how exactly the presence of the DMF code was causing generated files not to be saved when running test modules in different parts of the codebase
  • My best guess (mostly based on gut feeling, i.e. without any rigorous investigation) is that it might have something to do with the code formerly in idaes.core.dmf.util through manipulation of temporary directories and/or the working directory

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@lbianchi-lbl lbianchi-lbl self-assigned this Sep 11, 2024
@ksbeattie ksbeattie added the Priority:High High Priority Issue or PR label Sep 12, 2024
@ksbeattie ksbeattie requested review from aasgerr and ksbeattie and removed request for aasgerr September 12, 2024 18:39
@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 74.11765% with 22 lines in your changes missing coverage. Please review.

Project coverage is 76.83%. Comparing base (b6be02e) to head (33edfd6).
Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
idaes/conftest.py 73.80% 20 Missing and 2 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1488      +/-   ##
==========================================
- Coverage   76.84%   76.83%   -0.02%     
==========================================
  Files         376      376              
  Lines       61091    61170      +79     
  Branches    13505    13518      +13     
==========================================
+ Hits        46948    46999      +51     
- Misses      11762    11788      +26     
- Partials     2381     2383       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lbianchi-lbl lbianchi-lbl added the CI:run-integration triggers_workflow: Integration label Sep 19, 2024
@idaes-build idaes-build removed the CI:run-integration triggers_workflow: Integration label Sep 19, 2024
@lbianchi-lbl lbianchi-lbl marked this pull request as ready for review September 26, 2024 18:43
ksbeattie
ksbeattie approved these changes Sep 26, 2024
View reviewed changes
Copy link
Member

@ksbeattie ksbeattie left a comment

Choose a reason for hiding this comment

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

LGTM

@ksbeattie ksbeattie requested review from blnicho and removed request for MAZamarripa, andrewlee94, avdudchenko and bpaul4 September 26, 2024 18:46
@lbianchi-lbl lbianchi-lbl merged commit 9ee1fad into IDAES:main Sep 27, 2024
lbianchi-lbl added a commit that referenced this pull request Oct 1, 2024
@lbianchi-lbl
Add minimal pytest plugin to verify that generated files are cleaned …
47a0e18 
…up by tests (#1488)

* Add pytest plugin to verify that test modules clean up any generated file

* Format with Black and remove unused import

* Add docstring to make Pylint happy

* Format with Black

* Switch to per-test file checks and Python 3.9+ type hinting

* Apply fixtures to offending tests

* Try resolving next batch of failures

* Try addressing two failures

* Switch to using module-level fixture for tmp_path workdir

(cherry picked from commit 9ee1fad)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ksbeattie ksbeattie ksbeattie approved these changes

@blnicho blnicho blnicho approved these changes

Assignees

@lbianchi-lbl lbianchi-lbl

Labels

Priority:High High Priority Issue or PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lbianchi-lbl @codecov-commenter @ksbeattie @blnicho @idaes-build