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 non-ignored test, check all tests are executed #4262

Merged
merged 8 commits into from
Nov 26, 2020

Conversation

MarcoGorelli
Copy link
Contributor

In .github/workflows/pytest.yml there was --ignore=pymc3/tests/test_shape_handling instead of --ignore=pymc3/tests/test_shape_handling.py 😳 . So, fixed + pre-commit check to ensure all tests are executed added

types: [jupyter]
entry: python scripts/check_toc_is_complete.py
- id: check-no-tests-are-ignored
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the only new hook here, in the others I just sorted the keys

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call adding this hook 👍

ignored_tests = set(re.findall(r"(?<=--ignore=)(pymc3/tests.*\.py)", txt))
non_ignored_tests = set(re.findall(r"(?<!--ignore=)(pymc3/tests.*\.py)", txt))
assert (
ignored_tests <= non_ignored_tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: these are sets, so <= means "is a subset of"

@codecov
Copy link

codecov bot commented Nov 26, 2020

Codecov Report

Merging #4262 (fdbbc1d) into master (a6295a3) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4262      +/-   ##
==========================================
- Coverage   88.15%   88.14%   -0.02%     
==========================================
  Files          87       87              
  Lines       14243    14243              
==========================================
- Hits        12556    12554       -2     
- Misses       1687     1689       +2     
Impacted Files Coverage Δ
pymc3/backends/report.py 90.90% <0.00%> (-2.10%) ⬇️
pymc3/step_methods/hmc/base_hmc.py 98.33% <0.00%> (+0.83%) ⬆️

@MarcoGorelli
Copy link
Contributor Author

32m 8s ... nearly broke half-hour, GHA is really impressive

@MarcoGorelli MarcoGorelli requested a review from twiecki November 26, 2020 15:48
Copy link
Contributor

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

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

Good catch @MarcoGorelli! Thanks for the comments in the PR, makes it easier to review 👍
It seems that Travis is still doing its thing though -- I thought @twiecki had deactivated it 🤔

types: [jupyter]
entry: python scripts/check_toc_is_complete.py
- id: check-no-tests-are-ignored
Copy link
Contributor

Choose a reason for hiding this comment

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

Good call adding this hook 👍

@twiecki twiecki merged commit 8b3b701 into pymc-devs:master Nov 26, 2020
@twiecki
Copy link
Member

twiecki commented Nov 26, 2020

I think I disabled it, not sure why it still shows up...

@Spaak
Copy link
Member

Spaak commented Nov 27, 2020

In #4260 all GHA tests are passing, but status is still "waiting on Travis" which prevents a merge. Is it possible that the reason is that #4260 was filed before this one was merged? Any way to force-delete the Travis check?

@twiecki
Copy link
Member

twiecki commented Nov 27, 2020

No that has nothing to do with it, somehow travis is still installed but but non-functional but I can't find where.

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.

4 participants