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

Fix failing Nox tests session on Windows #337

Merged
merged 3 commits into from
Feb 21, 2022

Conversation

VKTB
Copy link
Contributor

@VKTB VKTB commented Feb 17, 2022

This PR will close #307

Description

Fixes the failing nox tests session on Windows.

Testing Instructions

  • Review code
  • Check GitHub Actions build
  • If icatdb Generator Script Consistency Test CI job fails, is this because of a deliberate change made to the script to change generated data (which isn't actually a problem) or is here an underlying issue with the changes made?
  • Review changes to test coverage
  • Does this change mean a new patch, minor or major version should be made? If so, does one of the commit messages feature fix:, feat: or BREAKING CHANGE: so a release is automatically made via GitHub Actions upon merge?
  • nox tests session should not fail
  • The version of the installed setuptools should be <58.0.0

@codecov
Copy link

codecov bot commented Feb 17, 2022

Codecov Report

Merging #337 (3416127) into master (d1292c1) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #337   +/-   ##
=======================================
  Coverage   92.69%   92.69%           
=======================================
  Files          43       43           
  Lines        3216     3216           
  Branches      321      321           
=======================================
  Hits         2981     2981           
  Misses        198      198           
  Partials       37       37           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1292c1...3416127. Read the comment docs.

@VKTB
Copy link
Contributor Author

VKTB commented Feb 17, 2022

From GitHub Actions
image

@VKTB VKTB marked this pull request as ready for review February 17, 2022 14:18
@VKTB VKTB requested a review from MRichards99 February 17, 2022 14:18
Copy link
Collaborator

@MRichards99 MRichards99 left a comment

Choose a reason for hiding this comment

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

Looks good, its always challenging supporting Windows! Just note the comment that we might as well make the change now

noxfile.py Outdated
@@ -95,6 +95,8 @@ def tests(session):
# version didn't fix the `2to3` issue when building Python ICAT, perhaps because
# Python ICAT isn't built on the downgraded version for some reason?
session.run("poetry", "run", "pip", "uninstall", "-y", "setuptools")
Copy link
Collaborator

Choose a reason for hiding this comment

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

For consistency, you might as well remove the poetry run from the uninstall - I did this on my machine without issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I removed it.

@VKTB VKTB requested a review from MRichards99 February 18, 2022 09:01
@VKTB VKTB merged commit 8b613da into master Feb 21, 2022
@VKTB VKTB deleted the fix-nox-test-session-on-windows-#307 branch February 21, 2022 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nox tests session failing to set up on Windows
2 participants