-
Notifications
You must be signed in to change notification settings - Fork 521
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
Workflow #245
base: master
Are you sure you want to change the base?
Workflow #245
Changes from 33 commits
ec875ac
d53f13e
b2b8354
1d9be56
6faec64
ab192a7
c59e469
a553765
3702287
a4d7006
6c8dfc2
0b442a3
fef43ca
85be207
60ef255
ffd12c7
e40a09e
e57c1b7
60676d3
bb13deb
e511b61
99f0e57
7cbd70a
4e8ba71
f56ef29
5405d1d
71a6517
36979bb
bd9be1e
cdaf0c9
8ea205f
979934d
c1b424c
4f0563e
d6df47b
31de88c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{ | ||
"env": { | ||
"browser": true, | ||
"es2021": true, | ||
"node": true | ||
}, | ||
"extends": "eslint:recommended", | ||
"parserOptions": { | ||
"ecmaVersion": "latest", | ||
"sourceType": "module" | ||
}, | ||
"overrides": [ | ||
{ | ||
"files": ["**/*.test.js"], | ||
"env": { "jest": true }, | ||
"plugins": ["jest"], | ||
"extends": ["plugin:jest/recommended"], | ||
"rules": { "jest/prefer-expect-assertions": "off" } | ||
}, | ||
{ | ||
"files": ["**/*.cy.js"], | ||
"env": { "cypress/globals": true }, | ||
"plugins": ["cypress"], | ||
"extends": ["plugin:cypress/recommended"], | ||
"rules": { | ||
"cypress/no-unnecessary-waiting": "off", | ||
"no-unused-vars": "off" | ||
} | ||
} | ||
], | ||
"rules": {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,8 +5,12 @@ permissions: | |
pull-requests: write | ||
|
||
on: | ||
pull_request_target: | ||
push: | ||
branches: | ||
- master | ||
pull_request: | ||
types: [opened, synchronize, reopened] | ||
workflow_dispatch: | ||
|
||
jobs: | ||
review: | ||
|
@@ -16,9 +20,34 @@ jobs: | |
uses: actions/checkout@v2 | ||
with: | ||
ref: ${{ github.event.pull_request.head.sha }} | ||
|
||
- name: Log environment info | ||
run: | | ||
echo "OS: $(uname -a)" | ||
echo "Shell: $SHELL" | ||
echo "GitHub Runner: ${{ runner.os }}" | ||
|
||
- name: Install dependencies | ||
run: npm install | ||
|
||
- name: Build project | ||
run: npm run build | ||
|
||
- name: Lint Code | ||
uses: github/super-linter@v4 | ||
env: | ||
DEFAULT_BRANCH: main | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
VALIDATE_JAVASCRIPT_ES: true | ||
ESLINT_CONFIG_FILE: .eslintrc.json | ||
|
||
- uses: fluxninja/openai-pr-reviewer@main | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} | ||
with: | ||
debug: false | ||
|
||
- name: Catch errors | ||
if: failure() | ||
run: echo "An error occurred during the workflow execution." | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The additional steps in the workflow are generally well structured. However, there are a few points to consider:
- name: Log environment info
run: |
echo "OS: $(uname -a)"
echo "Shell: $SHELL"
echo "GitHub Runner: ${{ runner.os }}"
- name: Install dependencies
- run: npm install
+ run: npm ci
- name: Catch errors
if: failure()
- run: echo "An error occurred during the workflow execution."
+ run: |
+ echo "An error occurred during the workflow execution."
+ exit 1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
_ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#!/bin/sh | ||
. "$(dirname "$0")/_/husky.sh" | ||
|
||
npx lint-staged |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"editor.defaultFormatter": "esbenp.prettier-vscode", | ||
"editor.formatOnSave": true, | ||
"editor.codeActionsOnSave": { | ||
"source.fixAll.eslint": true | ||
}, | ||
"eslint.validate": ["javascript"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
|
||
added 67 packages, and audited 386 packages in 6s | ||
|
||
55 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. | ||
Comment on lines
+1
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This hunk seems to be a log output from an -
- added 67 packages, and audited 386 packages in 6s
-
- 55 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's customary to add the Just missing the status badges here, instructions for them are at the end of this lesson: https://content.noroff.dev/workflow/automated-testing.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
## Badges | ||
|
||
[![Deploy static content to Pages](https://github.com/alexanderdyb/social-media-client/actions/workflows/pages.yml/badge.svg)](https://github.com/alexanderdyb/social-media-client/actions/workflows/pages.yml) | ||
|
||
[![Code Review](https://github.com/alexanderdyb/social-media-client/actions/workflows/gpt.yml/badge.svg)](https://github.com/alexanderdyb/social-media-client/actions/workflows/gpt.yml) | ||
|
||
[![Automated Unit Testing](https://github.com/alexanderdyb/social-media-client/actions/workflows/unit-test.yml/badge.svg)](https://github.com/alexanderdyb/social-media-client/actions/workflows/unit-test.yml) | ||
|
||
[![Automated E2E Testing](https://github.com/alexanderdyb/social-media-client/actions/workflows/e2e-test.yml/badge.svg)](https://github.com/alexanderdyb/social-media-client/actions/workflows/e2e-test.yml) | ||
|
||
## Getting started | ||
|
||
Before you start the project, make sure you have Node.js and npm installed on your machine. | ||
|
||
Install the Dependencies: | ||
|
||
npm i | ||
|
||
## Configuration | ||
|
||
- SASS for styling, along with a build script for compiling SASS to CSS. | ||
- Live-reloading with `live-server` for immediate feedback during development. | ||
- ESLint for linting JavaScript files. | ||
- Prettier for code formatting. | ||
- Husky and lint-staged for ensuring code quality through pre-commit hooks. | ||
- Unit testing with Jest. | ||
- End-to-end testing with Cypress. | ||
|
||
## Dependencies | ||
|
||
- Styling: sass, bootstrap-dark-5 | ||
- Development: live-server | ||
- Testing: jest, cypress | ||
- Linting & Formatting: eslint, prettier | ||
- Others: husky, lint-staged | ||
|
||
## Testing | ||
|
||
Run unit tests: | ||
npm run test-unit | ||
|
||
Run end-to-end tests: | ||
npm run test-e2e | ||
|
||
## To lint JavaScript files: | ||
|
||
npm run lint | ||
|
||
## To format code: | ||
|
||
npm run format |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"presets": [["@babel/preset-env", { "targets": { "node": "current" } }]] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
const { defineConfig } = require("cypress"); | ||
|
||
module.exports = defineConfig({ | ||
e2e: { | ||
// eslint-disable-next-line no-unused-vars | ||
setupNodeEvents(on, config) { | ||
// implement node event listeners here | ||
}, | ||
}, | ||
}); | ||
Comment on lines
+1
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new configuration for Cypress looks good. It's a good practice to use + const { defineConfig } = require("cypress");
+
+ module.exports = defineConfig({
+ e2e: {
+ // eslint-disable-next-line no-unused-vars
+ setupNodeEvents(on, config) {
+ // implement node event listeners here
+ },
+ },
+ }); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
describe("Login and Profile Access", () => { | ||
const email = "testusername21212@noroff.no"; | ||
const password = "Banner1212"; | ||
|
||
const loginPageUrl = "http://127.0.0.1:5500/"; | ||
const profilePageUrl = "http://127.0.0.1:5500/?view=profile&name=John_Doe"; | ||
|
||
it("should log in and access profile", () => { | ||
cy.visit(loginPageUrl); | ||
cy.wait(500); | ||
|
||
cy.get('#registerForm [data-auth="login"]').click(); | ||
cy.wait(500); | ||
|
||
cy.get("#loginForm input[name=email]").type(email); | ||
cy.get("#loginForm input[name=password]").type(password); | ||
|
||
cy.get("#loginForm button[type=submit]").click(); | ||
|
||
cy.url().should("eq", profilePageUrl); | ||
|
||
cy.contains("John_Doe").should("be.visible"); | ||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's good that you are checking the URL and visible content after login. However, it would be better to also check if the logout button or other user-specific elements are present on the page. This will ensure that the user is indeed logged in. // Add after line 22
cy.get('#logoutButton').should('be.visible'); |
||
}); | ||
}); | ||
Comment on lines
+1
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test case seems to be well written, but it's not a good practice to hardcode sensitive data like email and password directly in the test cases. Consider using environment variables or a configuration file that is not committed to the repository to store these values. This will also make your tests more flexible as you can change the test user without modifying the test code. - 2: const email = "testusername21212@noroff.no";
- 3: const password = "Banner1212";
+ 2: const email = Cypress.env('TEST_USER_EMAIL');
+ 3: const password = Cypress.env('TEST_USER_PASSWORD'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
describe("Invalid Login Attempt", () => { | ||
const invalidEmail = "invalid@noroff.no"; | ||
const invalidPassword = "wrongpassword"; | ||
|
||
const loginPageUrl = "http://127.0.0.1:5500/"; | ||
|
||
const errorMessageSelector = ".error-message"; | ||
|
||
it("should not allow login with invalid credentials and show an error", () => { | ||
cy.visit(loginPageUrl); | ||
cy.wait(500); | ||
|
||
cy.get('#registerForm [data-auth="login"]').click(); | ||
cy.wait(500); | ||
|
||
cy.get("#loginForm input[name=email]").type(invalidEmail); | ||
cy.get("#loginForm input[name=password]").type(invalidPassword); | ||
cy.get("#loginForm button[type=submit]").click(); | ||
|
||
cy.url().should("eq", loginPageUrl); | ||
|
||
cy.get(errorMessageSelector).should("be.visible"); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
describe("Logout Functionality", () => { | ||
const loginPageUrl = "http://127.0.0.1:5500/"; | ||
const profilePageUrl = "http://127.0.0.1:5500/?view=profile&name=John_Doe"; | ||
|
||
const email = "testusername21212@noroff.no"; | ||
const password = "Banner1212"; | ||
|
||
const logoutButtonSelector = '[data-auth="logout"]'; | ||
|
||
it("should log out the user when the logout button is clicked", () => { | ||
cy.visit(loginPageUrl); | ||
cy.wait(500); | ||
cy.get('#registerForm [data-auth="login"]').click(); | ||
cy.wait(500); | ||
|
||
cy.get("#loginForm input[name=email]").type(email); | ||
cy.get("#loginForm input[name=password]").type(password); | ||
cy.get("#loginForm button[type=submit]").click(); | ||
|
||
cy.url().should("eq", profilePageUrl); | ||
|
||
cy.get(logoutButtonSelector).click(); | ||
|
||
cy.url().should("eq", loginPageUrl); | ||
|
||
cy.window().its("localStorage").should("not.have.key", "token"); | ||
cy.window().its("localStorage").should("not.have.key", "profile"); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
describe("template spec", () => { | ||
it("passes", () => { | ||
cy.visit("https://example.cypress.io"); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"name": "Using fixtures to represent data", | ||
"email": "hello@cypress.io", | ||
"body": "Fixtures are a great way to mock data for responses to routes" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// *********************************************** | ||
// This example commands.js shows you how to | ||
// create various custom commands and overwrite | ||
// existing commands. | ||
// | ||
// For more comprehensive examples of custom | ||
// commands please read more here: | ||
// https://on.cypress.io/custom-commands | ||
// *********************************************** | ||
// | ||
// | ||
// -- This is a parent command -- | ||
// Cypress.Commands.add('login', (email, password) => { ... }) | ||
// | ||
// | ||
// -- This is a child command -- | ||
// Cypress.Commands.add('drag', { prevSubject: 'element'}, (subject, options) => { ... }) | ||
// | ||
// | ||
// -- This is a dual command -- | ||
// Cypress.Commands.add('dismiss', { prevSubject: 'optional'}, (subject, options) => { ... }) | ||
// | ||
// | ||
// -- This will overwrite an existing command -- | ||
// Cypress.Commands.overwrite('visit', (originalFn, url, options) => { ... }) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// *********************************************************** | ||
// This example support/e2e.js is processed and | ||
// loaded automatically before your test files. | ||
// | ||
// This is a great place to put global configuration and | ||
// behavior that modifies Cypress. | ||
// | ||
// You can change the location of this file or turn off | ||
// automatically serving support files with the | ||
// 'supportFile' configuration option. | ||
// | ||
// You can read more here: | ||
// https://on.cypress.io/configuration | ||
// *********************************************************** | ||
|
||
// Import commands.js using ES2015 syntax: | ||
import "./commands"; | ||
|
||
// Alternatively you can use CommonJS syntax: | ||
// require('./commands') |
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.
CodeRabbit
The changes made to the triggers for the GitHub Actions workflow are good. The workflow will now run on push events to the master branch, pull request events, and manual dispatches. This provides more flexibility and control over when the workflow is executed.