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

Change reference for jest-preset-angular/build/setupJest as per migration guide #10699

Merged
merged 1 commit into from
May 11, 2020

Conversation

adover
Copy link
Contributor

@adover adover commented May 9, 2020

Issue: #9072 - Affects addon-storyshots

What I did

Modified the path to jest-preset-angular/setupJest as per migration documentation https…://github.com/thymikee/jest-preset-angular/blob/5c071692d174d78639f952bf795bc06abd72d28c/CHANGELOG.md#migration-guide

How to test

This is testable by running within an Angular project which requires addon-storyshots

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@shilman shilman added addon: storyshots documentation patch:yes Bugfix & documentation PR that need to be picked to main branch labels May 11, 2020
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Thanks @adover !!!

@shilman
Copy link
Member

shilman commented May 12, 2020

@adover turns out this was a bad merge--there are more changes required to not break the storybook build. i'm going to revert this merge now.

@adover
Copy link
Contributor Author

adover commented May 12, 2020

Apologies @shilman . Do you have any information on this and I can attempt to do a proper job!

@shilman
Copy link
Member

shilman commented May 13, 2020

@adover no need to apologize. i shouldn't have merged it in the first place, and should have worked it out with you on the original PR, but missed the CI failure.

what i did:

  1. check out the repo
  2. apply your changes
  3. run yarn bootstrap --core then yarn test --core
  4. observe failed test due to an old version of jest-preset-angular
  5. upgraded jest-preset-angular
  6. observed some OTHER errors
  7. gave up and backed out the changes

If you can get it working, I'd be happy to merge again. Thanks for your help and patience!

@shilman shilman added angular bug and removed documentation patch:yes Bugfix & documentation PR that need to be picked to main branch labels Jun 3, 2020
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.

2 participants