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

Reverting workflow changes #2497

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
17baee2
configuring vitest
prayanshchh Nov 20, 2024
a117b91
migrating AddOn tests to vitest
prayanshchh Nov 21, 2024
0fc12b2
removing unnecessay alias
prayanshchh Nov 21, 2024
4ab8e9b
Merge branch 'develop-postgres' into migration_to_vitest
prayanshchh Nov 23, 2024
565c161
eslint fix
prayanshchh Nov 23, 2024
3d8cb16
suggestion #2
prayanshchh Nov 23, 2024
8708339
Merge branch 'develop-postgres' into migration_to_vitest
prayanshchh Nov 24, 2024
637f386
workflow setup
prayanshchh Nov 24, 2024
1c9ad17
Merge branch 'migration_to_vitest' of https://github.com/prayanshchh/…
prayanshchh Nov 24, 2024
9c24516
fix: tsdoc workflow
varshith257 Nov 24, 2024
79a8903
reformat comment to tsdoc fmt
varshith257 Nov 24, 2024
c1ef506
resolving jest, vitest workflow
prayanshchh Nov 25, 2024
297126a
Merge branch 'migration_to_vitest' of https://github.com/prayanshchh/…
prayanshchh Nov 25, 2024
265f5bd
Merge branch 'develop-postgres' into migration_to_vitest
prayanshchh Nov 25, 2024
1b9323d
lint error
prayanshchh Nov 25, 2024
1c34644
Merge branch 'migration_to_vitest' of https://github.com/prayanshchh/…
prayanshchh Nov 25, 2024
2cccb0b
add tsdoc comments
varshith257 Nov 25, 2024
b8d4c64
exit for workflow
prayanshchh Nov 25, 2024
dab3625
format yml fix
prayanshchh Nov 25, 2024
2f56784
fixing workflow
prayanshchh Nov 25, 2024
a7028fd
cleanup
varshith257 Nov 25, 2024
f0c45e0
ts error
prayanshchh Nov 25, 2024
220a555
Merge branch 'migration_to_vitest' of https://github.com/prayanshchh/…
prayanshchh Nov 25, 2024
2eac8c2
Update .eslintignore
varshith257 Nov 25, 2024
69f3217
tsconfig edit
prayanshchh Nov 25, 2024
05ae7b6
Merge branch 'migration_to_vitest' of https://github.com/prayanshchh/…
prayanshchh Nov 25, 2024
893d723
removing suggestions 2
prayanshchh Nov 25, 2024
98acc2f
Merge branch 'develop-postgres' into migration_to_vitest
varshith257 Nov 26, 2024
8bec841
Update vitest.config.ts
varshith257 Nov 26, 2024
11a6cb1
fmt
varshith257 Nov 26, 2024
c905d29
config update
prayanshchh Nov 26, 2024
a694964
merge
prayanshchh Nov 26, 2024
55ec1cc
ignoring .spec files for cov
prayanshchh Nov 26, 2024
99d8566
new suggestions
prayanshchh Nov 26, 2024
47f196e
merge reports
prayanshchh Nov 26, 2024
0e01e9a
Update vitest.config.ts
varshith257 Nov 26, 2024
96f9cd2
vitest run fix
prayanshchh Nov 26, 2024
8c8cdd9
Merge branch 'migration_to_vitest' of https://github.com/prayanshchh/…
prayanshchh Nov 26, 2024
1d933f4
report dir change
prayanshchh Nov 26, 2024
00c7b6b
report dir change 2
prayanshchh Nov 26, 2024
32df6a5
check workflow
prayanshchh Nov 26, 2024
0c23834
workflow update
prayanshchh Nov 26, 2024
bec94ad
fixing merge report
prayanshchh Nov 26, 2024
f4e1562
suggestion reverted
prayanshchh Nov 26, 2024
dbb8e2e
merge cov reports
prayanshchh Nov 26, 2024
ab6d03d
changing format of jest cov reports
prayanshchh Nov 26, 2024
6214b2f
Merge Coverage Reports Using lcov-result-merger
prayanshchh Nov 26, 2024
eea3f32
fixing merge report
prayanshchh Nov 26, 2024
220fc83
final changes
prayanshchh Nov 26, 2024
39d7676
final changes 2
prayanshchh Nov 26, 2024
0ac9523
reverting workflow changes
prayanshchh Nov 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
# Contains the PDF file of the Tag as JSON string, thus does not need to be linted
src/components/CheckIn/tagTemplate.ts
src/components/CheckIn/tagTemplate.ts
package.json
package-lock.json
tsconfig.json

71 changes: 40 additions & 31 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ jobs:
- name: Check formatting
if: steps.changed-files.outputs.only_changed != 'true'
run: npm run format:check

- name: Run formatting if check fails
if: failure()
run: npm run format
Expand All @@ -57,10 +57,10 @@ jobs:

- name: Check for linting errors in modified files
if: steps.changed-files.outputs.only_changed != 'true'
env:
env:
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
run: npx eslint ${CHANGED_FILES} && python .github/workflows/eslint_disable_check.py

- name: Check for TSDoc comments
run: npm run check-tsdoc # Run the TSDoc check script

Expand Down Expand Up @@ -89,7 +89,7 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Get Changed Unauthorized files
id: changed-unauth-files
uses: tj-actions/changed-files@v45
Expand Down Expand Up @@ -130,10 +130,10 @@ jobs:
*.password
*.secret
*.credentials

- name: List all changed unauthorized files
if: steps.changed-unauth-files.outputs.any_changed == 'true' || steps.changed-unauth-files.outputs.any_deleted == 'true'
env:
env:
CHANGED_UNAUTH_FILES: ${{ steps.changed-unauth-files.outputs.all_changed_files }}
run: |
for file in ${CHANGED_UNAUTH_FILES}; do
Expand All @@ -154,14 +154,14 @@ jobs:
uses: tj-actions/changed-files@v45

- name: Echo number of changed files
env:
env:
CHANGED_FILES_COUNT: ${{ steps.changed-files.outputs.all_changed_files_count }}
run: |
echo "Number of files changed: $CHANGED_FILES_COUNT"

- name: Check if the number of changed files is less than 100
if: steps.changed-files.outputs.all_changed_files_count > 100
env:
env:
CHANGED_FILES_COUNT: ${{ steps.changed-files.outputs.all_changed_files_count }}
run: |
echo "Error: Too many files (greater than 100) changed in the pull request."
Expand Down Expand Up @@ -201,15 +201,34 @@ jobs:

- name: Install Dependencies
run: npm install

- name: Get changed TypeScript files
id: changed-files
uses: tj-actions/changed-files@v45

- name: Run Jest Tests
if: steps.changed-files.outputs.only_changed != 'true'
env:
NODE_V8_COVERAGE: './coverage/jest'
run: |
npm run test -- --watchAll=false --coverage

- name: Run tests
- name: Run Vitest Tests
if: steps.changed-files.outputs.only_changed != 'true'
run: npm run test -- --watchAll=false --coverage
env:
NODE_V8_COVERAGE: './coverage/vitest'
run: |
npm run test:vitest:coverage
Comment on lines +209 to +221
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve test execution configuration

The current setup has several potential issues:

  1. Both Jest and Vitest tests use steps.changed-files.outputs.only_changed != 'true' which might skip tests unnecessarily
  2. Coverage directories are hardcoded which could cause conflicts

Consider these improvements:

       - name: Run Jest Tests
-        if: steps.changed-files.outputs.only_changed != 'true'
+        if: steps.changed-files.outputs.any_changed == 'true'
         env:
-          NODE_V8_COVERAGE: './coverage/jest'
+          NODE_V8_COVERAGE: './coverage/${{ github.run_id }}/jest'
         run: |
           npm run test -- --watchAll=false --coverage

       - name: Run Vitest Tests
-        if: steps.changed-files.outputs.only_changed != 'true'
+        if: steps.changed-files.outputs.any_changed == 'true'
         env:
-          NODE_V8_COVERAGE: './coverage/vitest'
+          NODE_V8_COVERAGE: './coverage/${{ github.run_id }}/vitest'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Run Jest Tests
if: steps.changed-files.outputs.only_changed != 'true'
env:
NODE_V8_COVERAGE: './coverage/jest'
run: |
npm run test -- --watchAll=false --coverage
- name: Run tests
- name: Run Vitest Tests
if: steps.changed-files.outputs.only_changed != 'true'
run: npm run test -- --watchAll=false --coverage
env:
NODE_V8_COVERAGE: './coverage/vitest'
run: |
npm run test:vitest:coverage
- name: Run Jest Tests
if: steps.changed-files.outputs.any_changed == 'true'
env:
NODE_V8_COVERAGE: './coverage/${{ github.run_id }}/jest'
run: |
npm run test -- --watchAll=false --coverage
- name: Run Vitest Tests
if: steps.changed-files.outputs.any_changed == 'true'
env:
NODE_V8_COVERAGE: './coverage/${{ github.run_id }}/vitest'
run: |
npm run test:vitest:coverage


- name: Merge Coverage Reports
if: steps.changed-files.outputs.only_changed != 'true'
run: |
mkdir -p coverage
if ! npx lcov-result-merger 'coverage/*/lcov.info' > 'coverage/lcov.info'; then
echo "Failed to merge coverage reports"
exit 1
fi

- name: TypeScript compilation for changed files
run: |
for file in ${{ steps.changed-files.outputs.all_files }}; do
Expand All @@ -224,12 +243,14 @@ jobs:
token: ${{ secrets.CODECOV_TOKEN }}
verbose: true
fail_ci_if_error: false
files: './coverage/lcov.info'
name: '${{env.CODECOV_UNIQUE_NAME}}'
fail_ci_if_error: false
Comment on lines 244 to +248
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix duplicate configuration in Codecov upload

There's a duplicate fail_ci_if_error key in the Codecov configuration.

Remove the duplicate:

       - name: Present and Upload coverage to Codecov as ${{env.CODECOV_UNIQUE_NAME}}
         uses: codecov/codecov-action@v4
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           verbose: true
-          fail_ci_if_error: false
           files: './coverage/lcov.info'
           name: '${{env.CODECOV_UNIQUE_NAME}}'
-          fail_ci_if_error: false

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 yamllint (1.35.1)

[error] 248-248: duplication of key "fail_ci_if_error" in mapping

(key-duplicates)


- name: Test acceptable level of code coverage
uses: VeryGoodOpenSource/very_good_coverage@v3
with:
path: './coverage/lcov.info'
path: "./coverage/lcov.info"
min_coverage: 95.0

Graphql-Inspector:
Expand All @@ -247,22 +268,23 @@ jobs:

- name: resolve dependency
run: npm install -g @graphql-inspector/cli

- name: Clone API Repository
run: |
# Retrieve the complete branch name directly from the GitHub context
FULL_BRANCH_NAME=${{ github.base_ref }}
echo "FULL_Branch_NAME: $FULL_BRANCH_NAME"

# Clone the specified repository using the extracted branch name
git clone --branch $FULL_BRANCH_NAME https://github.com/PalisadoesFoundation/talawa-api && ls -a
git clone --branch $FULL_BRANCH_NAME https://github.com/PalisadoesFoundation/talawa-api && ls -a

- name: Validate Documents
run: graphql-inspector validate './src/GraphQl/**/*.ts' './talawa-api/schema.graphql'

Start-App-Without-Docker:
name: Check if Talawa Admin app starts (No Docker)
runs-on: ubuntu-latest
name: Check if Talawa Admin app starts (No Docker)
runs-on: ubuntu-latest

needs: [Code-Quality-Checks, Test-Application]
if: github.actor != 'dependabot'
steps:
Expand All @@ -284,7 +306,6 @@ jobs:
run: |
npm run preview &
echo $! > .pidfile_prod

- name: Check if Production App is running
run: |
timeout=120
Expand All @@ -296,24 +317,20 @@ jobs:
echo "Still waiting for production app to start... ${timeout}s remaining"
fi
done

if [ $timeout -eq 0 ]; then
echo "Timeout waiting for production application to start"
exit 1
fi
echo "Production app started successfully"

- name: Stop Production App
run: |
if [ -f .pidfile_prod ]; then
kill "$(cat .pidfile_prod)"
fi

- name: Start Development App
run: |
npm run serve &
echo $! > .pidfile_dev

- name: Check if Development App is running
run: |
timeout=120
Expand All @@ -325,13 +342,11 @@ jobs:
echo "Still waiting for development app to start... ${timeout}s remaining"
fi
done

if [ $timeout -eq 0 ]; then
echo "Timeout waiting for development application to start"
exit 1
fi
echo "Development app started successfully"

- name: Stop Development App
if: always()
run: |
Expand All @@ -353,42 +368,36 @@ jobs:
with:
driver-opts: |
image=moby/buildkit:latest

- name: Build Docker image
run: |
set -e
echo "Building Docker image..."
docker build -t talawa-admin-app .
echo "Docker image built successfully"

- name: Run Docker Container
run: |
set -e
echo "Started Docker container..."
docker run -d --name talawa-admin-app-container -p 4321:4321 talawa-admin-app
echo "Docker container started successfully"

- name: Check if Talawa Admin App is running
run: |
timeout="${HEALTH_CHECK_TIMEOUT:-120}"
echo "Starting health check with ${timeout}s timeout"
while ! nc -z localhost 4321 && [ $timeout -gt 0 ]; do
sleep 1

timeout=$((timeout-1))
if [ $((timeout % 10)) -eq 0 ]; then
echo "Still waiting for app to start... ${timeout}s remaining"
fi
done

if [ $timeout -eq 0 ]; then
echo "Timeout waiting for application to start"
echo "Container logs:"
docker logs talawa-admin-app-container
exit 1
fi
echo "Port check passed, verifying health endpoint..."

- name: Stop Docker Container
if: always()
run: |
Expand All @@ -404,4 +413,4 @@ jobs:
if: github.event.pull_request.base.ref != 'develop-postgres'
run: |
echo "Error: Pull request target branch must be 'develop-postgres'. Please refer PR_GUIDELINES.md"
exit 1
exit 1
20 changes: 16 additions & 4 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
export default {
roots: ['<rootDir>/src'],
collectCoverageFrom: ['src/**/*.{ts,tsx}', '!src/index.tsx'],
collectCoverageFrom: [
'src/**/*.{ts,tsx}',
'!src/index.tsx',
'!node_modules',
'!dist',
'!**/*.{spec,test}.{js,jsx,ts,tsx}',
'!coverage/**',
'!**/index.{js,ts}',
'!**/*.d.ts',
'!src/test/**',
'!vitest.config.ts',],
// setupFiles: ['react-app-polyfill/jsdom'],
setupFiles: ['whatwg-fetch'],
setupFilesAfterEnv: ['<rootDir>/src/setupTests.ts'],
testMatch: [
'<rootDir>/src/**/__tests__/**/*.{js,jsx,ts,tsx}',
'<rootDir>/src/**/*.{spec,test}.{js,jsx,ts,tsx}',
'<rootDir>/src/**/*.test.{js,jsx,ts,tsx}',
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Update test pattern to include both .spec and .test files

The verification confirms that there are existing .spec files in the codebase that would be excluded by the new pattern:

  • src/Constant/constant.spec.ts
  • src/components/AddOn/AddOn.spec.tsx

The current change would break these test files by preventing them from being discovered by Jest. The pattern should be reverted to include both .spec and .test extensions.

🔗 Analysis chain

Verify impact on existing test files

The change from *.{spec,test} to *.test pattern might break existing spec files. This seems inconsistent with the PR objective of reverting workflow changes.

Let's check for any spec files that might be affected:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for .spec.{js,jsx,ts,tsx} files that might be affected
echo "Searching for spec files that might be excluded by the new pattern:"
fd -e spec.js -e spec.jsx -e spec.ts -e spec.tsx . src/

Length of output: 268

],
testEnvironment: 'jsdom',
transform: {
Expand All @@ -30,8 +40,8 @@ export default {
'^@dicebear/collection$':
'<rootDir>/scripts/__mocks__/@dicebear/collection.ts',
'\\.svg\\?react$': '<rootDir>/scripts/__mocks__/fileMock.js',
'\\.svg$': '<rootDir>/scripts/__mocks__/fileMock.js',
'^@pdfme/generator$': '<rootDir>/scripts/__mocks__/@pdfme/generator.ts'
'\\.svg$': '<rootDir>/scripts/__mocks__/fileMock.js',
'^@pdfme/generator$': '<rootDir>/scripts/__mocks__/@pdfme/generator.ts',
},
moduleFileExtensions: [
'web.js',
Expand Down Expand Up @@ -68,4 +78,6 @@ export default {
'<rootDir>/build/',
'<rootDir>/public/',
],
coverageDirectory: './coverage/jest',
coverageReporters: ['text', 'html', 'text-summary', 'lcov'],
};
Loading