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

Move from nosetest to pytest #2322

Merged
merged 5 commits into from
Jun 9, 2021
Merged

Move from nosetest to pytest #2322

merged 5 commits into from
Jun 9, 2021

Conversation

briehl
Copy link
Member

@briehl briehl commented Jun 8, 2021

Description of PR purpose/changes

The original motivation for this PR was found in the previous PR #2321 - Python code unit test coverage wasn't properly being uploaded to codecov. After a little poking, it seems that it's not being generated properly. Should be easy enough. I was also inspired to look at moving from nose to pytest at the same time.

So this is the first step (of likely many, TBA) in updating the Python test stack to be a little more modern. Nose is an ancient way of running Python tests. It has served us well, but it is long past time to move on. https://nose.readthedocs.io/en/latest/

Pytest is a framework that's used more commonly in the Python world, and at KBase in other projects. It has good support for test fixtures and mocking, and is under active community support. Best of all, it's basically a drop-in replacement! Seriously, the main change to get this running was to add it to requirements.txt, and change the call in scripts/narrative_backend_tests.sh. This exposed a number of minor warnings that were also changed - notably, any class in the test directory with Test as its first token is treated as a test, which got noisy. So the util.TestConfig class is now util.ConfigTests, for example.

A few other warnings popped up that were trivial to change, as well.

This does alter the requirements.txt file, so be sure to do a new installation locally in order to run tests.

  • [n/a] Added the Jira Ticket to the title of the PR (e.g. DATAUP-69 Adds a PR template)

Testing Instructions

  • Details for how to test the PR:
  • REINSTALL THE NARRATIVE! Making this loud and bold here.
  • Run tests as usual.
  • Tests pass locally and in GitHub Actions
  • Changes available by spinning up a local narrative and navigating to X to see Y

Dev Checklist:

  • My code follows the guidelines at https://sites.google.com/lbl.gov/trussresources/home?authuser=0
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • (Python) I have run Black and Flake8 on changed Python code manually or with a git precommit hook

Updating Version and Release Notes (if applicable)

@@ -216,7 +216,7 @@ def get_proxy_config():
if config_file:
_log.info("Configuring KBase logging from file '{}'".format(config_file))
else:
_log.warn(
_log.warning(
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a deprecation warning.

@@ -61,7 +61,7 @@ def __init__(self, input_file):
if not isinstance(input_file, IOBase):
input_file = open(str(input_file), "r")
try:
self._obj = yaml.load(input_file)
self._obj = yaml.load(input_file, Loader=yaml.SafeLoader)
Copy link
Member Author

Choose a reason for hiding this comment

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

Another deprecation warning. Really, both log_proxy and kblogging aren't in use, I don't believe, but this was safe enough.

@@ -6,7 +6,6 @@ jinja2==2.11.3
jsonschema==3.2.0
markupsafe==1.1.1
mock==4.0.3
nose>=1.1.2
Copy link
Member Author

Choose a reason for hiding this comment

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

Bye bye nose!

@@ -1,4 +1,4 @@
from ..util import TestConfig
from ..util import ConfigTests
Copy link
Member Author

Choose a reason for hiding this comment

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

The remaining changes go from TestConfig -> ConfigTests in the rest of these files.

Copy link
Collaborator

@ialarmedalien ialarmedalien left a comment

Choose a reason for hiding this comment

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

lgtm

@sonarcloud
Copy link

sonarcloud bot commented Jun 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Jun 9, 2021

Codecov Report

Merging #2322 (5f44a05) into develop (b599232) will increase coverage by 4.25%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2322      +/-   ##
===========================================
+ Coverage    12.91%   17.16%   +4.25%     
===========================================
  Files          415      451      +36     
  Lines        44581    48263    +3682     
===========================================
+ Hits          5757     8285    +2528     
- Misses       38824    39978    +1154     
Impacted Files Coverage Δ
src/biokbase/narrative/common/kblogging.py 82.94% <100.00%> (ø)
src/biokbase/narrative/common/log_proxy.py 48.20% <100.00%> (ø)
kbase-extension/static/kbase/js/common/unodep.js 42.62% <0.00%> (-6.56%) ⬇️
...-extension/static/kbase/js/util/bootstrapSearch.js 91.80% <0.00%> (-3.28%) ⬇️
src/biokbase/narrative/common/log_common.py 78.26% <0.00%> (ø)
src/biokbase/narrative/common/kvp.py 90.90% <0.00%> (ø)
src/biokbase/narrative/services/user.py 100.00% <0.00%> (ø)
src/biokbase/narrative/magics.py 0.00% <0.00%> (ø)
src/biokbase/narrative/widgetmanager.py 94.75% <0.00%> (ø)
src/biokbase/narrative/__init__.py 20.83% <0.00%> (ø)
... and 30 more

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 08b0c1a...5f44a05. Read the comment docs.

@briehl briehl merged commit 8b1c902 into develop Jun 9, 2021
@briehl briehl deleted the nose-to-pytest branch June 9, 2021 14: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.

None yet

2 participants