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

Register the translation mark with PyTest to suppress warnings #3618

Merged

Conversation

karlhigley
Copy link
Contributor

Pull Request

Description

This mark is used in the test suite to designate tests that should only
run during the translation tests and not as part of the regular notebook
tests. It works fine now, but results in warnings from PyTest at the end
of the test suite output about an unregistered mark. This change makes
the warnings go away, but otherwise doesn't change anything about the
functionality of the test suite.

Type of Change

Please mark options that are relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation (non-breaking change which adds documentation)
  • Improvement (non-breaking change that improves the performance or reliability of existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

This mark is used in the test suite to designate tests that should only
run during the translation tests and not as part of the regular notebook
tests. It works fine now, but results in warnings from PyTest at the end
of the test suite output about an unregistered mark. This change makes
the warnings go away, but otherwise doesn't change anything about the
functionality of the test suite.
@karlhigley karlhigley added the Type: Improvement 📈 Minor improvements not introducing a new feature or requiring a major refactor label May 27, 2020
@karlhigley karlhigley requested review from a team, shubham3121 and imskr and removed request for a team May 27, 2020 13:54
@karlhigley karlhigley self-assigned this May 27, 2020
@codecov
Copy link

codecov bot commented May 27, 2020

Codecov Report

Merging #3618 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3618      +/-   ##
==========================================
+ Coverage   94.64%   94.66%   +0.01%     
==========================================
  Files         160      160              
  Lines       17192    17194       +2     
==========================================
+ Hits        16272    16277       +5     
+ Misses        920      917       -3     
Impacted Files Coverage Δ
test/conftest.py 100.00% <100.00%> (ø)
syft/execution/plan.py 94.70% <0.00%> (+0.93%) ⬆️

Copy link
Member

@shubham3121 shubham3121 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 to me. 👍

@karlhigley
FYR: Notebook tests are failing because of examples/tutorials/advanced/Federated Dataset.ipynb fetches datasets from two urls which are now throwing 403.

@shubham3121 shubham3121 reopened this May 28, 2020
@shubham3121 shubham3121 merged commit 4194960 into OpenMined:master May 28, 2020
@karlhigley
Copy link
Contributor Author

@shubham3121 Thanks, didn't catch that! Looks like it might have been a temporary issue, but if it persists, we can open an issue. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement 📈 Minor improvements not introducing a new feature or requiring a major refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants