Ensure get() methods have correct type-hints #1681

JohnVillalovos · 2021-11-08T18:29:28Z

Fix classes which don't have correct 'get()' methods for classes
derived from GetMixin.

Add a unit test which verifies that classes have the correct return
type in their 'get()' method.

nejch

I like the idea so that people get a helpful message to guide them, because this can get quite complex for new contributors I think.

For me this (and test_mro.py) are kind of almost meta tests, so I wonder if we should move them together with test_dists.py or so (all the other modules in objects/ really test functions on gitlab resources).

tests/unit/objects/test_ensure_type_hints.py

JohnVillalovos · 2021-11-09T04:38:41Z

I like the idea so that people get a helpful message to guide them, because this can get quite complex for new contributors I think.

For me this (and test_mro.py) are kind of almost meta tests, so I wonder if we should move them together with test_dists.py or so (all the other modules in objects/ really test functions on gitlab resources).

Moving them works for me. I had put them in objects because those were the only part of the code-base that they checked.

If we move them, where do you think we should move them.

nejch · 2021-11-14T23:41:03Z

I like the idea so that people get a helpful message to guide them, because this can get quite complex for new contributors I think.
For me this (and test_mro.py) are kind of almost meta tests, so I wonder if we should move them together with test_dists.py or so (all the other modules in objects/ really test functions on gitlab resources).

Moving them works for me. I had put them in objects because those were the only part of the code-base that they checked.

If we move them, where do you think we should move them.

I was thinking maybe https://github.com/python-gitlab/python-gitlab/tree/main/tests/smoke along with test_dists.py since these have more of a meta/linting style to me. WDYT? I think "sanity/" might be more appropriate than "smoke/" now, but that was my fault 😀

tests/unit/objects/test_ensure_type_hints.py

JohnVillalovos · 2021-11-15T22:17:25Z

I was thinking maybe https://github.com/python-gitlab/python-gitlab/tree/main/tests/smoke along with test_dists.py since these have more of a meta/linting style to me. WDYT? I think "sanity/" might be more appropriate than "smoke/" now, but that was my fault 😀

I created a new tests/meta/ directory and put test_mro.py and test_ensure_type_hints.py in there.

codecov-commenter · 2021-11-15T22:28:15Z

Codecov Report

Merging #1681 (46773a8) into main (06184da) will decrease coverage by 0.32%.
The diff coverage is 81.42%.

@@            Coverage Diff             @@
##             main    #1681      +/-   ##
==========================================
- Coverage   91.88%   91.55%   -0.33%     
==========================================
  Files          75       75              
  Lines        4435     4571     +136     
==========================================
+ Hits         4075     4185     +110     
- Misses        360      386      +26

Flag	Coverage Δ
cli_func_v4	`80.83% <55.71%> (-0.82%)`	⬇️
py_func_v4	`80.94% <73.57%> (-0.25%)`	⬇️
unit	`83.02% <65.00%> (-0.59%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
gitlab/v4/objects/merge_requests.py	`84.67% <50.00%> (-0.52%)`	⬇️
gitlab/v4/objects/award_emojis.py	`89.65% <53.84%> (-10.35%)`	⬇️
gitlab/v4/objects/users.py	`95.79% <57.14%> (-2.71%)`	⬇️
gitlab/v4/objects/clusters.py	`93.93% <60.00%> (-6.07%)`	⬇️
gitlab/v4/objects/tags.py	`91.66% <60.00%> (-8.34%)`	⬇️
gitlab/v4/objects/container_registry.py	`81.48% <66.66%> (-2.52%)`	⬇️
gitlab/v4/objects/members.py	`94.33% <66.66%> (-5.67%)`	⬇️
gitlab/v4/objects/notes.py	`95.23% <76.47%> (-4.77%)`	⬇️
gitlab/v4/objects/events.py	`98.73% <93.33%> (-1.27%)`	⬇️
gitlab/v4/objects/audit_events.py	`100.00% <100.00%> (ø)`
... and 10 more

The 'test_mro.py' file is not really a unit test but more of a 'meta' check on the validity of the code base.

Fix classes which don't have correct 'get()' methods for classes derived from GetMixin. Add a unit test which verifies that classes have the correct return type in their 'get()' method.

JohnVillalovos marked this pull request as draft November 8, 2021 18:29

nejch reviewed Nov 8, 2021

View reviewed changes

tests/unit/objects/test_ensure_type_hints.py Outdated Show resolved Hide resolved

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch 3 times, most recently from 3df0d8a to 8fcec4b Compare November 9, 2021 04:35

JohnVillalovos changed the title ~~WIP: Ensure get() methods have correct type-hints~~ Ensure get() methods have correct type-hints Nov 9, 2021

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch from 8fcec4b to 196c6e1 Compare November 9, 2021 04:36

JohnVillalovos marked this pull request as ready for review November 9, 2021 04:36

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch from 196c6e1 to 5e54d67 Compare November 9, 2021 04:39

JohnVillalovos requested a review from nejch November 9, 2021 04:46

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch from 5e54d67 to c78e9de Compare November 14, 2021 19:35

nejch reviewed Nov 14, 2021

View reviewed changes

tests/unit/objects/test_ensure_type_hints.py Outdated Show resolved Hide resolved

tests/unit/objects/test_ensure_type_hints.py Outdated Show resolved Hide resolved

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch 2 times, most recently from 42726c9 to ae68668 Compare November 15, 2021 22:12

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch 2 times, most recently from b14212b to 7910780 Compare November 15, 2021 22:22

JohnVillalovos added 2 commits November 15, 2021 14:31

chore: create a 'tests/meta/' directory and put test_mro.py in it

94feb8a

The 'test_mro.py' file is not really a unit test but more of a 'meta' check on the validity of the code base.

chore: ensure get() methods have correct type-hints

46773a8

Fix classes which don't have correct 'get()' methods for classes derived from GetMixin. Add a unit test which verifies that classes have the correct return type in their 'get()' method.

JohnVillalovos force-pushed the jlvillal/mypy_ensure_type_hints branch from 7910780 to 46773a8 Compare November 15, 2021 22:31

JohnVillalovos requested a review from nejch November 16, 2021 20:01

nejch approved these changes Nov 16, 2021

View reviewed changes

nejch merged commit 0951989 into main Nov 16, 2021

nejch deleted the jlvillal/mypy_ensure_type_hints branch November 16, 2021 22:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ensure get() methods have correct type-hints #1681

Ensure get() methods have correct type-hints #1681

Uh oh!

JohnVillalovos commented Nov 8, 2021 •

edited

Loading

Uh oh!

nejch left a comment

Uh oh!

Uh oh!

JohnVillalovos commented Nov 9, 2021 •

edited

Loading

Uh oh!

nejch commented Nov 14, 2021

Uh oh!

Uh oh!

Uh oh!

JohnVillalovos commented Nov 15, 2021

Uh oh!

codecov-commenter commented Nov 15, 2021 •

edited

Loading

Uh oh!

Uh oh!

Ensure get() methods have correct type-hints #1681

Ensure get() methods have correct type-hints #1681

Uh oh!

Conversation

JohnVillalovos commented Nov 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nejch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JohnVillalovos commented Nov 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nejch commented Nov 14, 2021

Uh oh!

Uh oh!

Uh oh!

JohnVillalovos commented Nov 15, 2021

Uh oh!

codecov-commenter commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

JohnVillalovos commented Nov 8, 2021 •

edited

Loading

JohnVillalovos commented Nov 9, 2021 •

edited

Loading

codecov-commenter commented Nov 15, 2021 •

edited

Loading