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

changes for tox>=4.0 release #7384

Merged
merged 7 commits into from
Jan 26, 2023

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Dec 8, 2022

Closes #7383

This PR addresses the issue raised in #7383

Checklist for maintainers

  • added entry in CHANGES.rst within the relevant release section
  • updated or added relevant tests
  • updated relevant documentation
  • added relevant milestone
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below.
    How to run regression tests on a PR
  • Make sure the JIRA ticket is resolved properly

@zacharyburnett
Copy link
Collaborator Author

since Tox 4.0 is using PEP517, we run into the C extension issue noted in #7386

Copy link
Collaborator

@jdavies-st jdavies-st left a comment

Choose a reason for hiding this comment

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

I think you can unpin tox everywhere and just do

[testenv:check-dependencies]
description = verify that install_requires in setup.cfg has correct dependencies
usedevelop = false
extras =
commands =
    verify_install_requires

For some reason the new version of tox doesn't install the jwst wheel when usedevelop=true. Also, we had (by mistake) been using test dependencies in this job, which was a mistake, so the test extras needs to be turned off. The whole point of the job is to make sure the dependencies are accounted for without the test extras installed.

It's important that the pyargs job has usedevelop=false. The others need usedevelop=true, as that's where they are picking up installed code (built C binaries) when we run pytest.

@zacharyburnett zacharyburnett force-pushed the use_tox_4 branch 4 times, most recently from cf03b68 to 4e34084 Compare December 9, 2022 14:01
@zacharyburnett
Copy link
Collaborator Author

using package = wheel works as a PEP517-correct install with all of the C extensions; however, test-oldestdeps fails when the minimum_deps script tries to import jwst with pkg_resources

@zacharyburnett zacharyburnett force-pushed the use_tox_4 branch 3 times, most recently from 3e865e3 to 76ccdd1 Compare December 9, 2022 16:44
@zacharyburnett
Copy link
Collaborator Author

CI test success is blocked by #7386 and #7388

@zacharyburnett zacharyburnett force-pushed the use_tox_4 branch 4 times, most recently from da91d93 to c900411 Compare December 21, 2022 16:46
@zacharyburnett
Copy link
Collaborator Author

blocked by #7406

@zacharyburnett
Copy link
Collaborator Author

zacharyburnett commented Jan 25, 2023

rebased and formatting changes, plus added an intermediate step to cache CRDS before testing

@zacharyburnett zacharyburnett marked this pull request as ready for review January 25, 2023 18:17
@codecov
Copy link

codecov bot commented Jan 25, 2023

Codecov Report

Base: 78.65% // Head: 78.65% // No change to project coverage 👍

Coverage data is based on head (90aa708) compared to base (093ec3e).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7384   +/-   ##
=======================================
  Coverage   78.65%   78.65%           
=======================================
  Files         455      455           
  Lines       39168    39168           
=======================================
  Hits        30806    30806           
  Misses       8362     8362           
Flag Coverage Δ *Carryforward flag
nightly 78.64% <ø> (ø) Carriedforward from 093ec3e
unit 51.33% <ø> (-0.07%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@zacharyburnett zacharyburnett merged commit c5349ab into spacetelescope:master Jan 26, 2023
@zacharyburnett zacharyburnett deleted the use_tox_4 branch January 27, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation Continuous Integration (CI) and testing automation tools no-changelog-entry-needed testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check-dependencies started failing with no module named 'jwst'
3 participants