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

Add custom plot to e2e test #3646

Merged
merged 7 commits into from
Apr 10, 2023
Merged

Add custom plot to e2e test #3646

merged 7 commits into from
Apr 10, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Apr 5, 2023

Part of #3373

@julieg18 julieg18 self-assigned this Apr 5, 2023
@julieg18 julieg18 marked this pull request as ready for review April 5, 2023 16:48
@sroy3
Copy link
Contributor

sroy3 commented Apr 5, 2023

Maybe the plot creation could be its own test and one for the removal of same plot. That way we could test for the count to have updated as well.

@julieg18
Copy link
Contributor Author

julieg18 commented Apr 7, 2023

Thanks for the reviews! Added another plots test that check for adding and removing custom plot, plus ensured the created custom plot gets deleted. I believe we're ready for another round of reviews!

@julieg18 julieg18 requested a review from mattseddon April 7, 2023 02:07
await waitForDvcToFinish()
await webview.focus()

await waitForAllPlotsToRender(webview, 5)
Copy link
Member

Choose a reason for hiding this comment

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

[C] Seeing as you have made this helper you might want to make another one for checking the plots are not empty.

Copy link
Contributor Author

@julieg18 julieg18 Apr 10, 2023

Choose a reason for hiding this comment

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

Moved the code to a helper but ran into the expect-expect eslint warning. I disabled it with a comment for now but can revert the change or adjust our configuration if preferred!

Copy link
Member

Choose a reason for hiding this comment

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

You can use a comment to update the expect function names:

/* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "expectHeaders"] }] */

@codeclimate
Copy link

codeclimate bot commented Apr 10, 2023

Code Climate has analyzed commit b88a339 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 94.9% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit b4ea939 into main Apr 10, 2023
@julieg18 julieg18 deleted the add-custom-plots-to-e2e branch April 10, 2023 15: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.

3 participants