-
Notifications
You must be signed in to change notification settings - Fork 19
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
Tests should be refactored #67
Comments
@Sayli-Karnik Can I take this up, in case you're not? |
To be honest I'm not very convinced by the proposition of a «base class», but we can try to see how it goes. The idea is to go through every buckets/collections/groups/records client tests and see how much code is «repeated» between them. If there is a lot, let's go with the base class approach. Let us know what you find out ;) |
Hi all |
Well, if you get the thumbs up from @sahildua2305 , I believe you can go ahead and take it! |
Sure @zeddmaxx! Go ahead. It's all yours. |
@zeddmaxx Any progress on this? |
Accounting to no response from you, @zeddmaxx, I'd like to claim it back. /cc @almet @leplatrem |
@glasserc thanks! Btw, do we want to see how much code is repeated between corresponding tests for buckets/collections/records/groups? Or separately for Bucket tests and then for collection tests and so on? |
I think @almet's original goal was to share code that corresponds to the same test in buckets, collections, records, and groups. But any kind of DRYing up or refactoring could potentially be beneficial. |
@glasserc I noticed that there are a lot of tests in Buckets / Collections / Records / Groups which test the same functionality. For example - there are tests to verify that And that I think is intentional and necessary to make sure we have good test coverage. Making a base class and calling these tests indirectly using that class methods and just passing params necessary for new tests will make tests difficult to understand for new contributors. What do you think? |
I don't have a strong feeling about it, but I'm not as knowledgeable in this area as @leplatrem and @almet. If we think it's a bad idea, let's close the issue. |
Some tests would benefit from a refactor. I'm mostly thinking about the logic contained in the
_get_or_create
private method, which is not directly tested but via its callers.One solution here would be to create a base class with the behaviours we want to test and then derive multiple classes with the different inputs (bucket, collection and record).
The text was updated successfully, but these errors were encountered: