-
Notifications
You must be signed in to change notification settings - Fork 358
Environment testers #291
base: main
Are you sure you want to change the base?
Environment testers #291
Conversation
Sorry this I couldn't get the PR to show up quite the way I wanted. The intent is to reorganizing the tests so that all the test code isn't written four times. You can see how that works by looking at this file: https://github.com/plangrid/prop-types/blob/environment-testers/__tests__/arrayOf-test.js The tests are written in a callback which is then run once for each environment. The results look like this: Note that one outstanding issue is that the describe text for each test isn't completely accurate, but that is already the case with the copy/paste strategy. |
Primarily I wish to know if a change like this would be merged. If it would not be I don't want to spend my time moving the rest of the code around. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced this actually helps overall; using metaprogramming to create tests can make triaging failures harder.
I'm not saying there's no cost, but I'm pretty sure the benefit is worth it. Though it is difficult to tell, each of those existing test files tests arbitrary and subtly different things. It's 4x harder to write meaningful test code for any method, since most dev time is spent copying and pasting and working with the subtle differences of what goes where. Do I put the exact same code in each of the 4 files, just with different helpers? Many existing validator tests seem to feel that it isn't as important to test certain types of functionality in certain configurations. As a user of the package I just want to know that all the possible usages work as expected for each possible import style. Why would it not be valuable for the test code to be structured to match that expectation? |
@ljharb Could you help me any further? What would I have to do to convince you? If you can't be convinced, can you resolve for me whether the intention is to test all use cases in all environments, and if it isn't, I would like to know what the criteria for a usage being tested in a particular environment is. |
@conartist6 since you made this PR from a fork that's not your own, i don't think i'll be able to rebase this for you; can you rebase this? |
No description provided.