Jest test #390
Jest test #390
Conversation
Eslint & Prettier
|
WalkthroughThis pull request introduces a comprehensive set of changes aimed at improving code quality and maintainability. It includes the addition of ESLint, Prettier, and Husky configurations for enforcing coding standards, modifications to GitHub Actions workflows, introduction of Cypress end-to-end tests, and updates to various JavaScript modules. The changes also involve minor syntax adjustments like quote standardization and semicolon usage. 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. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (114)
- .eslintrc.json (1)
- .github/workflows/pages.yml (3)
- .husky/_/husky.sh (1)
- .husky/pre-commit (1)
- .prettierrc.json (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/support/commands.js (1)
- cypress/support/e2e.js (1)
- index.js (1)
- src/js/api/apiBase.js (1)
- src/js/api/auth/index.js (1)
- src/js/api/auth/login.js (1)
- src/js/api/auth/login.test.js (1)
- src/js/api/auth/logout.js (1)
- src/js/api/auth/logout.test.js (1)
- src/js/api/auth/register.js (1)
- src/js/api/auth/state.js (1)
- src/js/api/constants.js (1)
- src/js/api/headers.js (1)
- src/js/api/index.js (1)
- src/js/api/posts/comment.js (1)
- src/js/api/posts/create.js (1)
- src/js/api/posts/delete.js (1)
- src/js/api/posts/index.js (1)
- src/js/api/posts/react.js (1)
- src/js/api/posts/read.js (1)
- src/js/api/posts/update.js (1)
- src/js/api/profiles/delete.js (1)
- src/js/api/profiles/follow.js (1)
- src/js/api/profiles/index.js (1)
- src/js/api/profiles/read.js (1)
- src/js/api/profiles/unfollow.js (1)
- src/js/api/profiles/update.js (1)
- src/js/data/blank/post.js (1)
- src/js/index.js (1)
- src/js/listeners/auth/index.js (1)
- src/js/listeners/auth/login.js (1)
- src/js/listeners/auth/logout.js (1)
- src/js/listeners/auth/register.js (1)
- src/js/listeners/index.js (1)
- src/js/listeners/post/comment.js (2)
- src/js/listeners/post/index.js (1)
- src/js/listeners/post/reaction.js (2)
- src/js/listeners/profile/follow.js (2)
- src/js/listeners/profile/index.js (1)
- src/js/listeners/profile/unfollow.js (2)
- src/js/router/index.js (1)
- src/js/router/searchParams.js (1)
- src/js/storage/index.js (1)
- src/js/storage/load.js (1)
- src/js/storage/save.js (1)
- src/js/templates/comment/badge.js (1)
- src/js/templates/comment/comment.js (1)
- src/js/templates/comment/form.js (1)
- src/js/templates/comment/index.js (1)
- src/js/templates/index.js (1)
- src/js/templates/instance.js (1)
- src/js/templates/post/actions.js (2)
- src/js/templates/post/comments.js (1)
- src/js/templates/post/footer.js (1)
- src/js/templates/post/form.js (1)
- src/js/templates/post/header.js (1)
- src/js/templates/post/index.js (1)
- src/js/templates/post/loader.js (1)
- src/js/templates/post/media.js (1)
- src/js/templates/post/page.js (1)
- src/js/templates/post/reactions.js (1)
- src/js/templates/post/tabs.js (1)
- src/js/templates/post/tags.js (1)
- src/js/templates/post/thumbnail.js (1)
- src/js/templates/profile/button.js (1)
- src/js/templates/profile/followers.js (1)
- src/js/templates/profile/following.js (1)
- src/js/templates/profile/follows.js (1)
- src/js/templates/profile/index.js (1)
- src/js/templates/profile/page.js (1)
- src/js/templates/profile/thumbnail.js (1)
- src/js/tools/clear.js (1)
- src/js/tools/emojiSubset.js (1)
- src/js/tools/index.js (1)
- src/js/ui/auth.js (1)
- src/js/ui/footer.js (1)
- src/js/ui/header.js (1)
- src/js/ui/index.js (1)
- src/js/ui/modals.js (1)
- src/js/ui/renderView.js (1)
- src/js/views/index.js (1)
- src/js/views/postList.js (1)
- src/js/views/postPage.js (2)
- src/js/views/profileList.js (1)
- src/js/views/profilePage.js (1)
- src/test/baseline.test.js (1)
- src/test/es6.js (1)
- src/test/es6.test.js (1)
Files ignored due to filter (4)
- babel.config.json
- cypress/fixtures/example.json
- 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 (99)
- .eslintrc.json
- .github/workflows/pages.yml
- .husky/_/husky.sh
- .husky/pre-commit
- .prettierrc.json
- cypress.config.js
- cypress/e2e/2-advanced-examples/aliasing.cy.js
- cypress/e2e/2-advanced-examples/connectors.cy.js
- cypress/e2e/2-advanced-examples/misc.cy.js
- cypress/e2e/2-advanced-examples/navigation.cy.js
- cypress/e2e/2-advanced-examples/querying.cy.js
- cypress/e2e/2-advanced-examples/spies_stubs_clocks.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
- index.js
- src/js/api/apiBase.js
- src/js/api/auth/index.js
- src/js/api/auth/login.test.js
- src/js/api/auth/logout.js
- src/js/api/auth/logout.test.js
- src/js/api/auth/register.js
- src/js/api/auth/state.js
- src/js/api/constants.js
- src/js/api/headers.js
- src/js/api/index.js
- src/js/api/posts/comment.js
- src/js/api/posts/create.js
- src/js/api/posts/delete.js
- src/js/api/posts/index.js
- src/js/api/posts/react.js
- src/js/api/posts/read.js
- src/js/api/posts/update.js
- src/js/api/profiles/delete.js
- src/js/api/profiles/follow.js
- src/js/api/profiles/index.js
- src/js/api/profiles/read.js
- src/js/api/profiles/unfollow.js
- src/js/data/blank/post.js
- src/js/index.js
- src/js/listeners/auth/index.js
- src/js/listeners/auth/login.js
- src/js/listeners/auth/logout.js
- src/js/listeners/auth/register.js
- src/js/listeners/index.js
- src/js/listeners/post/comment.js
- src/js/listeners/post/index.js
- src/js/listeners/post/reaction.js
- src/js/listeners/profile/follow.js
- src/js/listeners/profile/index.js
- src/js/router/index.js
- src/js/router/searchParams.js
- src/js/storage/index.js
- src/js/storage/load.js
- src/js/storage/save.js
- src/js/templates/comment/badge.js
- src/js/templates/comment/comment.js
- src/js/templates/comment/form.js
- src/js/templates/comment/index.js
- src/js/templates/index.js
- src/js/templates/instance.js
- src/js/templates/post/actions.js
- src/js/templates/post/footer.js
- src/js/templates/post/form.js
- src/js/templates/post/header.js
- src/js/templates/post/index.js
- src/js/templates/post/loader.js
- src/js/templates/post/media.js
- src/js/templates/post/page.js
- src/js/templates/post/reactions.js
- src/js/templates/post/tabs.js
- src/js/templates/post/tags.js
- src/js/templates/post/thumbnail.js
- src/js/templates/profile/button.js
- src/js/templates/profile/followers.js
- src/js/templates/profile/following.js
- src/js/templates/profile/follows.js
- src/js/templates/profile/index.js
- src/js/templates/profile/page.js
- src/js/templates/profile/thumbnail.js
- src/js/tools/clear.js
- src/js/tools/emojiSubset.js
- src/js/tools/index.js
- src/js/ui/auth.js
- src/js/ui/footer.js
- src/js/ui/header.js
- src/js/ui/index.js
- src/js/ui/modals.js
- src/js/ui/renderView.js
- src/js/views/index.js
- src/js/views/postList.js
- src/js/views/postPage.js
- src/js/views/profileList.js
- src/js/views/profilePage.js
- src/test/baseline.test.js
- src/test/es6.js
Review comments generated (20)
- Review: 5
- LGTM: 15
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.
| try { | ||
| await unfollowProfile(name); | ||
| location.reload() | ||
| location.reload(); | ||
| } catch { | ||
| return alert("There was a problem unfollowing this profile"); | ||
| return alert('There was a problem unfollowing this profile'); | ||
| } | ||
| } |
There was a problem hiding this comment.
There's a potential issue with the use of location.reload(). This will refresh the entire page which might not be the best user experience. Consider updating only the necessary components instead. Also, using alert for error handling is not recommended as it blocks the browser and is generally considered bad UX. Consider displaying the error message in a non-blocking manner.
- await unfollowProfile(name);
- location.reload();
+ const result = await unfollowProfile(name);
+ if (result.success) {
+ // Update the UI component here instead of reloading the whole page.
+ } else {
+ // Display error message in a non-blocking way.
+ }
- return alert('There was a problem unfollowing this profile');
+ // Display error message in a non-blocking way.| import { apiPath } from '../constants.js'; | ||
| import { headers } from '../headers.js'; | ||
| import { save } from '../../storage/index.js'; | ||
|
|
||
| export async function login(email, password) { | ||
| const response = await fetch(`${apiPath}/social/auth/login`, { | ||
| method: "post", | ||
| method: 'post', | ||
| body: JSON.stringify({ email, password }), | ||
| headers: headers("application/json") | ||
| }) | ||
| headers: headers('application/json'), | ||
| }); | ||
|
|
||
| if (response.ok) { | ||
| const profile = await response.json() | ||
| save("token", profile.accessToken) | ||
| delete profile.accessToken | ||
| save("profile", profile) | ||
| return profile | ||
| const profile = await response.json(); | ||
| save('token', profile.accessToken); | ||
| delete profile.accessToken; | ||
| save('profile', profile); | ||
| return profile; | ||
| } | ||
|
|
||
| throw new Error(response.statusText) | ||
| } No newline at end of file | ||
| throw new Error(response.statusText); | ||
| } |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [1-22]
The changes made to the login function are mostly stylistic, such as changing double quotes to single quotes. However, there is a significant change in error handling. Previously, an error was thrown regardless of whether the response was successful or not. Now, an error is only thrown if the response is not ok (i.e., if the HTTP status code is not in the 200-299 range). This is a good practice because it allows for more accurate error handling based on the actual outcome of the fetch request.
One potential issue is that the save function might throw an error if the accessToken or profile is null or undefined. It would be better to add some null checks before calling save.
if (response.ok) {
const profile = await response.json();
+ if (!profile || !profile.accessToken) {
+ throw new Error('Invalid profile data');
+ }
save('token', profile.accessToken);
delete profile.accessToken;
+ if (!profile) {
+ throw new Error('Invalid profile data');
+ }
save('profile', profile);
return profile;
}This way, we can ensure that we're not trying to save null or undefined values, which could potentially cause errors down the line.
| import { wait } from './es6'; | ||
|
|
||
| test("It resolves with 'abc' value after 1 second", async () => { | ||
| const start = Date.now(); | ||
| const data = await wait(1000, 'abc'); | ||
| expect(data).toEqual('abc'); | ||
| const end = Date.now(); | ||
| expect(end - start).toBeGreaterThanOrEqual(1000); | ||
| }); |
There was a problem hiding this comment.
The test case seems to be well written and covers the functionality of the wait function. It checks if the function resolves with the expected value after a specified delay and also verifies that the elapsed time is at least equal to the delay. However, it's important to note that this test might introduce flakiness due to its dependency on real-time delays. Consider mocking the timers using Jest's Timer Mocks for more reliable tests.
+ jest.useFakeTimers();
test("It resolves with 'abc' value after 1 second", async () => {
const start = Date.now();
+ jest.advanceTimersByTime(1000);
const data = await wait(1000, 'abc');
expect(data).toEqual('abc');
const end = Date.now();
expect(end - start).toBeGreaterThanOrEqual(1000);
});Remember to clear all mocks in an afterEach block to ensure isolation between tests.
+ afterEach(() => {
+ jest.clearAllMocks();
+ });| export async function updateProfileImage(avatar) { | ||
| const me = profile() | ||
| const me = profile(); | ||
|
|
||
| const response = await fetch(`${apiPath}/social/profiles/${me.name}`, { | ||
| method: "put", | ||
| method: 'put', | ||
| body: JSON.stringify({ ...me, avatar }), | ||
| headers: headers("application/json") | ||
| }) | ||
| headers: headers('application/json'), | ||
| }); | ||
|
|
||
| if (response.ok) { | ||
| return await response.json() | ||
| return await response.json(); | ||
| } |
There was a problem hiding this comment.
There is a potential issue with the updateProfileImage function. If the fetch call fails (for example, due to network issues), it will throw an exception that is not caught here. This could lead to unhandled promise rejections. It would be better to wrap the fetch call in a try-catch block to handle any exceptions that might occur.
export async function updateProfileImage(avatar) {
const me = profile();
try {
const response = await fetch(`${apiPath}/social/profiles/${me.name}`, {
method: 'put',
body: JSON.stringify({ ...me, avatar }),
headers: headers('application/json'),
});
if (response.ok) {
return await response.json();
}
} catch (error) {
console.error(error);
throw error;
}|
|
||
| if (response.ok) { | ||
| return await response.json() | ||
| return await response.json(); | ||
| } | ||
|
|
There was a problem hiding this comment.
The newline at the end of the file is missing. While this won't cause any functional issues, it's generally considered good practice to end files with a newline. This is because some tools like Git and certain text editors expect or require this newline and may not function correctly without it.
- \ No newline at end of file
+ 
Creating unit tests for login and logout functions
Summary by CodeRabbit
npm cifor dependency installation.waitfunction that returns a promise resolving after a specified delay.