-
-
Notifications
You must be signed in to change notification settings - Fork 872
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
Changes from all commits
17baee2
a117b91
0fc12b2
4ab8e9b
565c161
3d8cb16
8708339
637f386
1c9ad17
9c24516
79a8903
c1ef506
297126a
265f5bd
1b9323d
1c34644
2cccb0b
b8d4c64
dab3625
2f56784
a7028fd
f0c45e0
220a555
2eac8c2
69f3217
05ae7b6
893d723
98acc2f
8bec841
11a6cb1
c905d29
a694964
55ec1cc
99d8566
47f196e
0e01e9a
96f9cd2
8c8cdd9
1d933f4
00c7b6b
32df6a5
0c23834
bec94ad
f4e1562
dbb8e2e
ab6d03d
6214b2f
eea3f32
220fc83
39d7676
0ac9523
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 |
---|---|---|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 | ||
|
||
|
@@ -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 | ||
|
@@ -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 | ||
|
@@ -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." | ||
|
@@ -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 | ||
|
||
- 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 | ||
|
@@ -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
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. Fix duplicate configuration in Codecov upload There's a duplicate 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
🧰 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: | ||
|
@@ -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: | ||
|
@@ -284,7 +306,6 @@ jobs: | |
run: | | ||
npm run preview & | ||
echo $! > .pidfile_prod | ||
|
||
- name: Check if Production App is running | ||
run: | | ||
timeout=120 | ||
|
@@ -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 | ||
|
@@ -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: | | ||
|
@@ -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: | | ||
|
@@ -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 |
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}', | ||
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. 💡 Codebase verification Update test pattern to include both The verification confirms that there are existing
The current change would break these test files by preventing them from being discovered by Jest. The pattern should be reverted to include both 🔗 Analysis chainVerify impact on existing test files The change from Let's check for any spec files that might be affected: 🏁 Scripts executedThe 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: { | ||
|
@@ -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', | ||
|
@@ -68,4 +78,6 @@ export default { | |
'<rootDir>/build/', | ||
'<rootDir>/public/', | ||
], | ||
coverageDirectory: './coverage/jest', | ||
coverageReporters: ['text', 'html', 'text-summary', 'lcov'], | ||
}; |
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.
🛠️ Refactor suggestion
Improve test execution configuration
The current setup has several potential issues:
steps.changed-files.outputs.only_changed != 'true'
which might skip tests unnecessarilyConsider these improvements:
📝 Committable suggestion