Workflow CA #272
Workflow CA #272
Conversation
Create main.yml
Create e2e-test.yml
Create unit-test.yml
|
WalkthroughThis pull request introduces a comprehensive testing suite with unit and E2E tests, automated workflows via GitHub Actions, and ESLint configuration. It also includes a mock implementation of localStorage for testing purposes, a new exported function with its test case, and Git hooks setup using Husky. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (37)
- .eslintrc.json (1)
- .github/workflows/e2e-test.yml (1)
- .github/workflows/unit-test.yml (1)
- .gitignore (1)
- .husky/_/husky.sh (1)
- .husky/pre-commit (1)
- 6 (1)
- README.md (1)
- cypress.config.js (1)
- cypress/e2e/1-getting-started/todo.cy.js (1)
- cypress/e2e/2-advanced-examples/actions.cy.js (1)
- cypress/e2e/2-advanced-examples/aliasing.cy.js (1)
- cypress/e2e/2-advanced-examples/assertions.cy.js (1)
- cypress/e2e/2-advanced-examples/connectors.cy.js (1)
- cypress/e2e/2-advanced-examples/cookies.cy.js (1)
- cypress/e2e/2-advanced-examples/cypress_api.cy.js (1)
- cypress/e2e/2-advanced-examples/files.cy.js (1)
- cypress/e2e/2-advanced-examples/location.cy.js (1)
- cypress/e2e/2-advanced-examples/misc.cy.js (1)
- cypress/e2e/2-advanced-examples/navigation.cy.js (1)
- cypress/e2e/2-advanced-examples/network_requests.cy.js (1)
- cypress/e2e/2-advanced-examples/querying.cy.js (1)
- cypress/e2e/2-advanced-examples/spies_stubs_clocks.cy.js (1)
- cypress/e2e/2-advanced-examples/storage.cy.js (1)
- cypress/e2e/2-advanced-examples/traversal.cy.js (1)
- cypress/e2e/2-advanced-examples/utilities.cy.js (1)
- cypress/e2e/2-advanced-examples/viewport.cy.js (1)
- cypress/e2e/2-advanced-examples/waiting.cy.js (1)
- cypress/e2e/2-advanced-examples/window.cy.js (1)
- cypress/e2e/E2E-test/e2e.test.js (1)
- cypress/support/commands.js (1)
- cypress/support/e2e.js (1)
- es6.js (1)
- es6.test.js (1)
- src/js/index.js (1)
- src/js/mocks/localStorage.mock.js (1)
- src/js/storage/save.test.js (1)
Files ignored due to filter (28)
- babel.config.json
- cypress/fixtures/example.json
- cypress/fixtures/profile.json
- cypress/fixtures/users.json
- cypress/screenshots/2-advanced-examples/misc.cy.js/my-image.png
- cypress/videos/1-getting-started/todo.cy.js.mp4
- cypress/videos/2-advanced-examples/actions.cy.js.mp4
- cypress/videos/2-advanced-examples/aliasing.cy.js.mp4
- cypress/videos/2-advanced-examples/assertions.cy.js.mp4
- cypress/videos/2-advanced-examples/connectors.cy.js.mp4
- cypress/videos/2-advanced-examples/cookies.cy.js.mp4
- cypress/videos/2-advanced-examples/cypress_api.cy.js.mp4
- cypress/videos/2-advanced-examples/files.cy.js.mp4
- cypress/videos/2-advanced-examples/location.cy.js.mp4
- cypress/videos/2-advanced-examples/misc.cy.js.mp4
- cypress/videos/2-advanced-examples/navigation.cy.js.mp4
- cypress/videos/2-advanced-examples/network_requests.cy.js.mp4
- cypress/videos/2-advanced-examples/querying.cy.js.mp4
- cypress/videos/2-advanced-examples/spies_stubs_clocks.cy.js.mp4
- cypress/videos/2-advanced-examples/storage.cy.js.mp4
- cypress/videos/2-advanced-examples/traversal.cy.js.mp4
- cypress/videos/2-advanced-examples/utilities.cy.js.mp4
- cypress/videos/2-advanced-examples/viewport.cy.js.mp4
- cypress/videos/2-advanced-examples/waiting.cy.js.mp4
- cypress/videos/2-advanced-examples/window.cy.js.mp4
- cypress/videos/3-testing/no.wikipedia.cy.js.mp4
- package-lock.json
- package.json
Files not summarized due to errors (1)
- cypress/e2e/2-advanced-examples/actions.cy.js (diff tokens exceeds limit)
Files skipped from review due to trivial changes (23)
- .eslintrc.json
- .github/workflows/e2e-test.yml
- .github/workflows/unit-test.yml
- .gitignore
- .husky/_/husky.sh
- .husky/pre-commit
- cypress/e2e/2-advanced-examples/aliasing.cy.js
- cypress/e2e/2-advanced-examples/cookies.cy.js
- cypress/e2e/2-advanced-examples/files.cy.js
- cypress/e2e/2-advanced-examples/location.cy.js
- cypress/e2e/2-advanced-examples/misc.cy.js
- cypress/e2e/2-advanced-examples/navigation.cy.js
- cypress/e2e/2-advanced-examples/network_requests.cy.js
- cypress/e2e/2-advanced-examples/querying.cy.js
- cypress/e2e/2-advanced-examples/storage.cy.js
- cypress/e2e/2-advanced-examples/utilities.cy.js
- cypress/e2e/2-advanced-examples/waiting.cy.js
- cypress/e2e/2-advanced-examples/window.cy.js
- cypress/support/commands.js
- cypress/support/e2e.js
- es6.js
- src/js/index.js
- src/js/storage/save.test.js
Review comments generated (17)
- Review: 5
- LGTM: 12
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| - Added a new files called "localStorage.mock.js" + "save.test.js" | ||
| - Unit testing for storing token on login with valid credentials and for clearing token when logout is clicked | ||
| - Added E2E testing for login, login error and logout | ||
| - Updated content of README.md |
There was a problem hiding this comment.
It's a good practice to always end your files with a newline. This is because some tools like cat, awk, or vi might not process the file correctly if it doesn't end with a newline.
- Updated content of README.md
+ \ No newline at end of file| /// <reference types="cypress" /> | ||
|
|
||
| context('Connectors', () => { | ||
| beforeEach(() => { | ||
| cy.visit('https://example.cypress.io/commands/connectors') | ||
| }) | ||
|
|
||
| it('.each() - iterate over an array of elements', () => { | ||
| // https://on.cypress.io/each | ||
| cy.get('.connectors-each-ul>li') | ||
| .each(($el, index, $list) => { | ||
| console.log($el, index, $list) | ||
| }) | ||
| }) | ||
|
|
||
| it('.its() - get properties on the current subject', () => { | ||
| // https://on.cypress.io/its | ||
| cy.get('.connectors-its-ul>li') | ||
| // calls the 'length' property yielding that value | ||
| .its('length') | ||
| .should('be.gt', 2) | ||
| }) | ||
|
|
||
| it('.invoke() - invoke a function on the current subject', () => { | ||
| // our div is hidden in our script.js | ||
| // $('.connectors-div').hide() | ||
| cy.get('.connectors-div').should('be.hidden') | ||
|
|
||
| // https://on.cypress.io/invoke | ||
| // call the jquery method 'show' on the 'div.container' | ||
| cy.get('.connectors-div').invoke('show') | ||
|
|
||
| cy.get('.connectors-div').should('be.visible') | ||
| }) | ||
|
|
||
| it('.spread() - spread an array as individual args to callback function', () => { | ||
| // https://on.cypress.io/spread | ||
| const arr = ['foo', 'bar', 'baz'] | ||
|
|
||
| cy.wrap(arr).spread((foo, bar, baz) => { | ||
| expect(foo).to.eq('foo') | ||
| expect(bar).to.eq('bar') | ||
| expect(baz).to.eq('baz') | ||
| }) | ||
| }) | ||
|
|
||
| describe('.then()', () => { | ||
| it('invokes a callback function with the current subject', () => { | ||
| // https://on.cypress.io/then | ||
| cy.get('.connectors-list > li') | ||
| .then(($lis) => { | ||
| expect($lis, '3 items').to.have.length(3) | ||
| expect($lis.eq(0), 'first item').to.contain('Walk the dog') | ||
| expect($lis.eq(1), 'second item').to.contain('Feed the cat') | ||
| expect($lis.eq(2), 'third item').to.contain('Write JavaScript') | ||
| }) | ||
| }) | ||
|
|
||
| it('yields the returned value to the next command', () => { | ||
| cy.wrap(1) | ||
| .then((num) => { | ||
| expect(num).to.equal(1) | ||
|
|
||
| return 2 | ||
| }) | ||
| .then((num) => { | ||
| expect(num).to.equal(2) | ||
| }) | ||
| }) | ||
|
|
||
| it('yields the original subject without return', () => { | ||
| cy.wrap(1) | ||
| .then((num) => { | ||
| expect(num).to.equal(1) | ||
| // note that nothing is returned from this callback | ||
| }) | ||
| .then((num) => { | ||
| // this callback receives the original unchanged value 1 | ||
| expect(num).to.equal(1) | ||
| }) | ||
| }) | ||
|
|
||
| it('yields the value yielded by the last Cypress command inside', () => { | ||
| cy.wrap(1) | ||
| .then((num) => { | ||
| expect(num).to.equal(1) | ||
| // note how we run a Cypress command | ||
| // the result yielded by this Cypress command | ||
| // will be passed to the second ".then" | ||
| cy.wrap(2) | ||
| }) | ||
| .then((num) => { | ||
| // this callback receives the value yielded by "cy.wrap(2)" | ||
| expect(num).to.equal(2) | ||
| }) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
The code looks good overall, but there's a potential issue with the use of console.log in production code. It's generally not a good practice to leave console.log statements in your production code as it can expose sensitive information and clutter the console output.
.each(($el, index, $list) => {
- console.log($el, index, $list)
+ // console.log($el, index, $list) // Uncomment for debugging purposes
})|
|
||
| added 52 packages, and audited 371 packages in 3s | ||
|
|
||
| 53 packages are looking for funding | ||
| run `npm fund` for details | ||
|
|
||
| 4 high severity vulnerabilities | ||
|
|
||
| To address issues that do not require attention, run: | ||
| npm audit fix | ||
|
|
||
| To address all issues (including breaking changes), run: | ||
| npm audit fix --force | ||
|
|
||
| Run `npm audit` for details. |
There was a problem hiding this comment.
This hunk seems to be the output of an npm install or similar command. It's generally not a good practice to include such outputs in your codebase, as they can quickly become outdated and may contain system-specific information. Instead, consider adding instructions in your README file about how to handle package installation and addressing potential vulnerabilities.
- 1:
- 2: added 52 packages, and audited 371 packages in 3s
- 3:
- 4: 53 packages are looking for funding
- 5: run `npm fund` for details
- 6:
- 7: 4 high severity vulnerabilities
- 8:
- 9: To address issues that do not require attention, run:
- 10: npm audit fix
- 11:
- 12: To address all issues (including breaking changes), run:
- 13: npm audit fix --force
- 14:
- 15: Run `npm audit` for details.|
|
||
| it("should log out successfully", () => { | ||
| // Log in first (you can use a custom Cypress command for login) | ||
| cy.login("validUsername", "validPassword"); // Implement the login command as per your app | ||
|
|
||
| // Locate and click the logout button | ||
| cy.get(".logout-button").click(); // Adjust the selector for the logout button | ||
|
|
||
| // Assert that the user is redirected to the login page or a success message is displayed | ||
| cy.url().should("include", "/login"); // Adjust the URL accordingly | ||
| }); | ||
| }); |
There was a problem hiding this comment.
In the test case "should log out successfully", it's assumed that the login operation is successful. However, if the login operation fails for some reason (e.g., due to a change in the application's login mechanism), this test will also fail, even though the logout functionality might be working correctly. To make your tests more independent and reliable, consider mocking the login state instead of actually logging in.
it("should log out successfully", () => {
// Log in first (you can use a custom Cypress command for login)
- 32: cy.login("validUsername", "validPassword"); // Implement the login command as per your app
+ 32: cy.setLoginState(); // Mock the login state
// Locate and click the logout button
cy.get(".logout-button").click(); // Adjust the selector for the logout button
// Assert that the user is redirected to the login page or a success message is displayed
cy.url().should("include", "/login"); // Adjust the URL accordingly
});| cy.url().should("include", "/login"); // Adjust the URL accordingly | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
It's a good practice to end files with a newline character. This helps with version control systems like Git and tools like cat, sed, awk, etc. which may not behave as you expect without a newline at the end of the file.
});
+ 42: 
| // please read our getting started guide: | ||
| // https://on.cypress.io/introduction-to-cypress | ||
|
|
||
| describe('example to-do app', () => { |
There was a problem hiding this comment.
You can remove all of the example files that ship with Cypress
There was a problem hiding this comment.
You should not commit video files as these will change often each time the test is run. Please delete these and change your settings to avoid producing videos in cypress.config.
| @@ -0,0 +1,9 @@ | |||
| import { wait } from "./es6"; | |||
There was a problem hiding this comment.
You should remove all example/demo files before submission.
My final pull request for Workflow CA
Summary by CodeRabbit
localStoragefor testing purposes, allowing us to isolate and test storage-related functionalities effectively.