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: Changed setUp and tearDown from class method to object method #1094

Conversation

onefloid
Copy link
Contributor

@onefloid onefloid commented Apr 4, 2024

The methods setUp() and tearDown() are not meant to be class methods. I have double checked the docs and can't find any reason to declare them as class methods.

All tests run without failures after these changes.

This PR contains:

  • Removed @classmethod decorators for setUp() and tearDown()
  • Renamed the argument from cls to self

Copy link
Collaborator

@MariusWirtz MariusWirtz left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning up the tests and getting rid of technical debt.

The code looks good. Please just squash the two commits into one.

@onefloid onefloid force-pushed the issue/changed-test-setUp-and-tearDown-to-object-methods branch from 1cecc00 to 4ed02ac Compare April 5, 2024 07:56
@onefloid onefloid marked this pull request as draft April 5, 2024 08:00
@MariusWirtz MariusWirtz marked this pull request as ready for review April 5, 2024 09:05
@MariusWirtz MariusWirtz merged commit f3317d0 into cubewise-code:master Apr 5, 2024
@onefloid
Copy link
Contributor Author

onefloid commented Apr 5, 2024

Sorry, the merge was a little bit to fast. There is still a small syntax error in AnnotationService Test. I will provide an patch in an extra PR soonish

@onefloid
Copy link
Contributor Author

onefloid commented Apr 5, 2024

Sorry, the merge was a little bit to fast. There is still a small syntax error in AnnotationService Test. I will provide an patch in an extra PR soonish

I discovered that only my local copy was affected. Everything is fine in Github. So there is nothing to patch.

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.

2 participants