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

ProjectConfig.name to ProjectConfig.id #10592

Closed
wants to merge 9 commits into from
Closed

Conversation

jhalak27
Copy link

@jhalak27 jhalak27 commented Oct 5, 2020

Changed name: string to id: string in file jest/packages/jest-types/src/Config.ts
and in jest/packages/jest-config/src/normalize.ts

Summary

Closes #10013

Test plan

@jeysal
Copy link
Contributor

jeysal commented Oct 5, 2020

Hi again, looking good so far but seems you'll also need to update some test - see CI failures

@jhalak27
Copy link
Author

jhalak27 commented Oct 5, 2020

Yeah, actually. I wanted to ask whether I should change in those files also? I was afraid to change without asking as these are the test files which needs change.

@jeysal
Copy link
Contributor

jeysal commented Oct 5, 2020

Yeah, the tests should also check for the new name 🙃

@jeysal
Copy link
Contributor

jeysal commented Oct 5, 2020

Let me know once you have this passing and ready (or of course if you need any further help) :)

@jhalak27
Copy link
Author

@jeysal Hey, I am unable to figure out the remaining errors. Could you help in that?

@jeysal
Copy link
Contributor

jeysal commented Oct 13, 2020

Hi, so if you look at e.g. the multiProjectRunner.test.ts, you'll see that there is a config object that still has name: 'something' in it. Something like this is probably the problem for all of the tests.
If you change that to id and ideally also add an as Config.InitialOptions to the config object so that the type checker can prevent something like this from happening in the future, that should work :)

@jhalak27
Copy link
Author

@jeysal I am not able to find the objects which still has name. Is there anything I am missing?

@jeysal
Copy link
Contributor

jeysal commented Oct 19, 2020

For the example I mentioned, it's right there in multiProjectRunner.test.ts 🙈 just search for name in that file and you'll find some 😅

@jhalak27
Copy link
Author

I did change every name to id in multiProjectRunner.test.ts in the last commit I made. I search all again, but couldn't find. There are just displayName. sweat_smile

@jeysal
Copy link
Contributor

jeysal commented Oct 19, 2020

Oh sorry, must have missed that commit

@jeysal
Copy link
Contributor

jeysal commented Oct 19, 2020

So the reason why that test is still not working is probably that groupOptions in jest-config/src/index still has name.
Some other tests still fail or cause warnings because they specify name instead of id somewhere. E.g. lots of warnings from jest-runtime are caused because its tests all use the createRuntime.js helper which still specifies name.
I'm afraid I can't point you to every single occurrence (because if I did, I'd have to invest more time than if I just made the changes myself on the go), but if you can get most of the things out of the way I can help with any few remaining errors that are particularly hard to find or hidden in complex tests.

@github-actions
Copy link

github-actions bot commented May 9, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename ProjectConfig.name to ProjectConfig.id
4 participants