-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Test suite changes #1254
Test suite changes #1254
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/bv5aecgyt |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 79ff495:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question , did you move the tests to mobile cases or keeping both ?
Keeping both. I simply copied the same tests (with a few minor changes) over to mobile. Mobile is the equvalient of adding cy.viewport('iphone-x`)
cy.visit('localhost:3000`) to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see #1254 (comment)
I am seeing some removal of code from the old tests. Can you give me some details about that ? thanks |
Sure, no worries! Its mainly because I've wrapped the entire test in a For example: |
It's not as big a deal for test code, but I would prefer to avoid duplicate code if possible, having both mobile and desktop tests import from a shared file. |
Gonna close this PR for now. I'll have a look again once again in the near future (~1 week) |
@mohammedsahl Another thing you can do instead of closing is leaving it open but mark it as a "draft" PR in the right sidebar. Looks like this: Also when making a new PR it gives you the option to create it up front as a "draft" PR. Sorry for lag on my end, It's tax time and have been working on business stuff related to that. |
Summary
This PR aims to duplicate the current sidebar e2e tests for the desktop over to mobile. In addition there is a small test for #1243
What kind of change does this PR introduce? (check at least one)
Integration tests added. Complete overhaul of
cypress/snapshots/sidebar
snapshots.If changing the UI of default theme, please provide the before/after screenshot:
Does this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
You have tested in the following browsers: (Providing a detailed version will be better.)
Other information:
Adding
describe
orcontext
orit
changes the filename of the snapshot. This causes test to fail.In order to use
describe
andcontext
, I had to delete all existing snapshots and re-runnpm run test:e2e
.I verified if the new snapshots generated were correct through manual inspection. In addition I carried out a second verification by running
npm run test:e2e
twice in a row, ensuring all tests pass.I expect some of the e2e tests checks to fail for this PR