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

Update tests to use .name() and rely on wrapper id #51

Merged
merged 2 commits into from
Sep 28, 2023
Merged

Conversation

clavedeluna
Copy link
Contributor

@clavedeluna clavedeluna commented Sep 28, 2023

Overview

I noticed we forgot to update tests when we changed the codemod API. I think this is the way to go to encourage us to use .name() instead of accessing the metadata attr directly.

Secondly, now that we have a codemod wrapper, and in fact the wrapper is what we use to report id in codetf results, then I was able to remove the base_codemod id class attr. It just needed an update in integration tests.

@codecov-commenter
Copy link

Codecov Report

Merging #51 (540089b) into main (df11542) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #51   +/-   ##
=======================================
  Coverage   96.04%   96.04%           
=======================================
  Files          41       41           
  Lines        1593     1593           
=======================================
  Hits         1530     1530           
  Misses         63       63           

@clavedeluna clavedeluna changed the title Update tests to use .name() Update tests to use .name() and rely on wrapper id Sep 28, 2023
@@ -1,11 +1,12 @@
# pylint: disable=no-member
# pylint: disable=no-member, protected-access, attribute-defined-outside-init
Copy link
Member

Choose a reason for hiding this comment

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

❤️ but also maybe still a code smell that indicates we can clean up the class hierarchy a bit.

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 actually because this integration test code is:

  1. calling on _codemods. I didn't want to write a class method just for tests calling it
  2. bc pylint isn't good at understanding that defining something inside setup_method in pytest is kinda the equivalent of defining it in an init method.

@clavedeluna clavedeluna merged commit 4bcf73d into main Sep 28, 2023
6 checks passed
@clavedeluna clavedeluna deleted the name-api branch September 28, 2023 14:54
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.

3 participants