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

PR Template Requests Running Tests But Gives No Instruction #17703

Closed
wesleyboar opened this issue Mar 13, 2022 · 5 comments
Closed

PR Template Requests Running Tests But Gives No Instruction #17703

wesleyboar opened this issue Mar 13, 2022 · 5 comments

Comments

@wesleyboar
Copy link
Contributor

wesleyboar commented Mar 13, 2022

Update: Resolved. There is a link—outside the PR description—to the contribution guide. Also, #17713.


Is your feature request related to a problem? Please describe
Yes. My PR is #17702, but anyone opening a PR for a UI change may be requested to run Jest or Chromatic tests.

There is no instruction for testing, and the process was too much for so small a change, yet the request was still made.

Describe the solution you'd like
A link in the Pull Request template to instructions for how to perform tests requested.

I support requesting testing, but I do not support the unexpected trouble when trying to test. See "Additional context".

Describe alternatives you've considered

  1. Researching on my own how to run the requested tests. See "Additional context".
  2. Improving the wording of the PR template to disregard testing for a minuscule changes.

Are you able to assist to bring the feature to reality?
Yes, but I am too angry at the moment to promise to assist beyond creating this ticket.

Additional context

I took many steps to run the test suite, but eventually failed. Click for details.
  1. Clone my fork of the repo.

  2. Run yarn install.

  3. Run yarn jest.

  4. See tests fail because dev dependencies were not installed.

  5. Fail to install all dependencies because I was using a command for Yarn Classic.

  6. Determine my yarn version... version 3.

  7. Fail to learn how to install all dev dependencies with Yarn 3.

  8. Learn how to downgrade to Yarn Classic.

  9. Install all dev dependencies.

  10. Be asked for which version to use for every @storybook/... dev dependency! 1

    Couldn't find any versions for "@storybook/svelte" that matches "workspace:*"
    ? Please choose a version of "@storybook/svelte" from this list: 
    

    I chose 6.5.0-alpha.47 for every options, because I am impatient and annoyed.

  11. Be lambasted with many warnings.

  12. Receive one error:
    error An unexpected error occurred: "expected workspace package to exist for \"react-router-dom\"".

  13. Run yarn jest.

  14. Receive more reports of missing dependencies. See snippet.

  15. Give up!

Footnotes

  1. Why is a known working version of each dependency not pinned?!

@wesleyboar
Copy link
Contributor Author

wesleyboar commented Mar 13, 2022

I see now that Chromatic tests run automatically for a PR. Super, thanks for the unnecessary workout (see "Additional context" in issue description). I still suggest some instruction. I tried to be a good FOSS citizen, but now feel like a disenchanted consumer.

@yannbf
Copy link
Member

yannbf commented Mar 13, 2022

Hey @tacc-wbomar, I greatly appreciate your contribution to this project and I'm sorry you went through a bad experience with testing.

We have a contributor's guide of which you can find linked in our CONTRIBUTING.md file and that ends up pointing to this page. Did you see that before? That page contains instructions to properly install and test the repository. We use yarn workspaces and the dependencies are locally built and mapped, so you shouldn't have to select any of our dependency versions from the npm registry.

Unfortunately the monorepo has quite some complexity so it's not that trivial to find out how to test certain changes based on what files you touched. In your case, the CLI templates are only used in the E2E tests, which we have instructions for here.

I'm sure the PR template could be improved, and I'd love to hear your thoughts to avoid having other contributors going through similar frustrations. Thank you!

@wesleyboar
Copy link
Contributor Author

wesleyboar commented Mar 14, 2022

@yannbf,

I did not see that link before. I now see it on the right under "Helpful resources". That link would have likely resolved all of my issues. Thank you for patient, kind, and thorough reply to my angry issue.

I am now unsure whether the PR template should change. Users more familiar with the GitHub contribution process would likely know about those links. I may open a PR anyway—to add a statement that points new users to outside the PR description field for helpful resource links—and the team can decide whether it could help.

Thanks again.

@yannbf
Copy link
Member

yannbf commented Mar 14, 2022

No worries @tacc-wbomar! I totally understand your frustration and I hope others don't go through similar experience.

I see value in improving the PR template and I'm looking forward to seeing yet another contribution from you!

@shilman
Copy link
Member

shilman commented Mar 31, 2022

Jeepers creepers!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.52 containing PR #17713 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants