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

Environment testers #291

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

conartist6
Copy link
Contributor

No description provided.

@conartist6
Copy link
Contributor Author

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:
Screen Shot 2019-08-30 at 3 53 21 PM

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.

@conartist6
Copy link
Contributor Author

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.

Copy link
Collaborator

@ljharb ljharb left a 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.

@conartist6
Copy link
Contributor Author

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?

@conartist6
Copy link
Contributor Author

@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.

@ljharb
Copy link
Collaborator

ljharb commented Nov 29, 2021

@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?

@ljharb ljharb marked this pull request as draft December 22, 2021 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants