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

Fix error in DashboardNavigation stories #5585

Closed
aaemnnosttv opened this issue Jul 21, 2022 · 4 comments
Closed

Fix error in DashboardNavigation stories #5585

aaemnnosttv opened this issue Jul 21, 2022 · 4 comments
Labels
Good First Issue Good first issue for new engineers P1 Medium priority QA: Eng Requires specialized QA by an engineer Type: Infrastructure Engineering infrastructure & tooling

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Jul 21, 2022

Bug Description

VRT runs currently show the following ReferenceError that happens several times during a test run:

Unexpected error while loading ./components/DashboardNavigation/index.stories.js: ReferenceError: describe is not defined

This is from the stories module importing a helper from a Jest test file which includes calls to describe as a side-effect. This is provided automatically by Jest so it errors as undefined in VRT runs.

Steps to reproduce

  1. Run VRT or inspect the output of a run
  2. See the error above

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The describe is not defined error should not be raised during VRT runs

Implementation Brief

  • We need to import the setupDefaultChips() function into the story file from a non-Jest file so that describe isn't called as a side-effect.
  • Inside assets/js/components/DashboardNavigation:
    • Create a new file named test-utils.js. In this file:
      • Add the default file header comments. The description of the file should be DashboardNavigation test utility functions..
      • Copy over the setupDefaultChips() function from index.test.js to this file (including relevant imports and JSDoc comments).
    • In index.test.js, import setupDefaultChips() from test-utils.js. Remove unnecessary dependencies
    • In index.stories.js, import setupDefaultChips() from test-utils.js instead of index.test.js.

Test Coverage

  • N/A

QA Brief

  • QA:Eng - The Reference Error mentioned in the AC will not show up in VRT test logs.
    • Check it in the PR run.
    • Check locally as well.

Changelog entry

  • Fix js errors in the storybook stories.
@aaemnnosttv aaemnnosttv added P1 Medium priority Type: Infrastructure Engineering infrastructure & tooling labels Jul 21, 2022
@aaemnnosttv aaemnnosttv self-assigned this Jul 21, 2022
@aaemnnosttv aaemnnosttv added the Good First Issue Good first issue for new engineers label Jul 21, 2022
@aaemnnosttv aaemnnosttv removed their assignment Jul 21, 2022
@nfmohit nfmohit assigned nfmohit and unassigned nfmohit Jul 27, 2022
@techanvil techanvil self-assigned this Jul 29, 2022
@techanvil
Copy link
Collaborator

Hey @nfmohit, this IB looks good. One minor tweak I would suggest is to use the filename test-utils.js rather than utils.js, just to make it clear that what's in the file is related to non-production code only.

@techanvil techanvil assigned nfmohit and unassigned techanvil Jul 29, 2022
@nfmohit
Copy link
Collaborator

nfmohit commented Aug 1, 2022

Hey @nfmohit, this IB looks good. One minor tweak I would suggest is to use the filename test-utils.js rather than utils.js, just to make it clear that what's in the file is related to non-production code only.

Very valid suggestion. I've updated the IB. Thank you @techanvil!

@nfmohit nfmohit assigned techanvil and unassigned nfmohit Aug 1, 2022
@techanvil
Copy link
Collaborator

Thanks @nfmohit!

IB ✅

@techanvil techanvil removed their assignment Aug 2, 2022
@kuasha420 kuasha420 self-assigned this Aug 13, 2022
@kuasha420 kuasha420 removed their assignment Aug 14, 2022
@eugene-manuilov eugene-manuilov self-assigned this Aug 15, 2022
eugene-manuilov added a commit that referenced this issue Aug 15, 2022
…tion-story

Move `setupDefaultChips` to a seperate file.
@eugene-manuilov eugene-manuilov removed their assignment Aug 15, 2022
@mohitwp mohitwp added the QA: Eng Requires specialized QA by an engineer label Aug 16, 2022
@hussain-t hussain-t self-assigned this Aug 16, 2022
@hussain-t
Copy link
Collaborator

QA:Eng Verified ✅

  • Verified it in the PR run.
  • Verified running VRT locally as well.

@hussain-t hussain-t removed their assignment Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Good first issue for new engineers P1 Medium priority QA: Eng Requires specialized QA by an engineer Type: Infrastructure Engineering infrastructure & tooling
Projects
None yet
Development

No branches or pull requests

8 participants