ember exam test partitioning #1505

avzz-19 · 2024-12-12T12:19:12Z

No description provided.

coderabbitai

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 909d300 and f09b02b.

📒 Files selected for processing (2)

.github/workflows/test.yaml (2 hunks)
package.json (1 hunks)

🔇 Additional comments (2)

.github/workflows/test.yaml (1)

8-8: Verify npm dependencies installation in setup action

The build job correctly uploads the built application as an artifact for test jobs. However, we should verify that the setup action includes npm dependencies installation.

Also applies to: 19-23

✅ Verification successful

Setup action correctly handles npm dependencies installation

The setup-app action includes proper npm dependencies installation with:

Node.js setup with npm caching
Dependency caching using package-lock.json hash
npm ci installation when cache is not available

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if setup-app action includes npm install
cat .github/actions/setup-app/action.yml

Length of output: 111

Script:

#!/bin/bash
# Let's check the workflow file to see the setup steps
cat .github/workflows/test.yaml

Length of output: 1725

Script:

#!/bin/bash
# Let's check if setup-app action exists in a different location
fd "setup-app" .github/

Length of output: 50

Script:

#!/bin/bash
# Check contents of the setup-app action directory
ls -la .github/actions/setup-app/

Length of output: 204

Script:

#!/bin/bash
# Check the contents of the setup-app action file
cat .github/actions/setup-app/action.yaml

Length of output: 595

package.json (1)

28-30: Verify test split configurations and ember-exam setup

The test split configuration raises some questions:

Why are unit tests split into 3 parts while integration/acceptance tests are split into 6 parts? Consider documenting the rationale or adjusting based on:
- Test execution times
- Test suite sizes
- Available CI resources
Verify that ember-exam is properly configured for test type filtering:

✅ Verification successful

Based on the output, I can now generate the final response as I have enough information about the test distribution in the codebase:

Split configuration aligns well with test distribution

The current split configuration is appropriate based on the test distribution in the codebase:

Unit tests (split=3): ~150 test files, mostly smaller and faster tests focusing on individual components
Integration tests (split=6): ~200 test files with component integration scenarios
Acceptance tests (split=6): ~30 test files with full user journey scenarios that typically take longer to execute

The higher split count (6) for integration and acceptance tests is justified because:

Integration tests involve component interactions and DOM rendering, making them slower
Acceptance tests simulate full user journeys with browser interactions, being the slowest category

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check ember-exam configuration in ember-cli-build.js
cat ember-cli-build.js | grep -A 10 'ember-exam'

# Check test types distribution
rg -g "tests/**/*-test.{js,ts}" "^(import|module)\s.*unit" -c
rg -g "tests/**/*-test.{js,ts}" "^(import|module)\s.*integration" -c
rg -g "tests/**/*-test.{js,ts}" "^(import|module)\s.*acceptance" -c

Length of output: 32846

.github/workflows/test.yaml

cloudflare-workers-and-pages · 2024-12-12T12:23:39Z

Deploying irenestaging with Cloudflare Pages

Latest commit:	`7b7ae1c`
Status:	✅ Deploy successful!
Preview URL:	https://bc4bb3f7.irenestaging.pages.dev
Branch Preview URL:	https://test-partitioning.irenestaging.pages.dev

View logs

cypress · 2024-12-12T12:30:09Z

Irene Run #543

Run Properties: Failed #543 • 0124992296 ℹ️: Merge f09b02bd3dd7eb4589ee954317693f75a52e3b4e into 909d300289d87e403a338f0470fa...

Project	`Irene`
Branch Review	`test-partitioning`
Run status	`Failed #543`
Run duration	`05m 48s`
Commit	`0124992296 ℹ️: Merge f09b02bd3dd7eb4589ee954317693f75a52e3b4e into 909d300289d87e403a338f0470fa...`
Committer	`Avi Shah`
View all properties for this run ↗︎

Test results
Failures	`2`
Flaky	`0`
Pending	`0`
Skipped	`0`
Passing	`30`
View all changes introduced in this branch ↗︎

Tests for review

dynamic-scan.spec.ts • 1 failed test

View Output

Test		Artifacts
Dynamic Scan > it tests dynamic scan for an apk file: 58062		`Test Replay` `Screenshots`

service-account.spec.ts • 1 failed test

View Output

Test		Artifacts
Service Account > should create service account with expiry for some projects & delete it		`Test Replay` `Screenshots`

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb57a9d and c6ff8b9.

📒 Files selected for processing (6)

.github/actions/setup-app/action.yaml (1 hunks)
.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (5)

.github/actions/setup-app/action.yaml
app/components/organization-namespace/request-status/index.ts
package.json
app/components/upload-app/status/details/index.ts
app/components/organization-namespace/approval-status/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

74-74: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

76-76: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

78-78: context "env" is not allowed here. available contexts are "github", "inputs", "needs", "vars". see https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability for more details

(expression)

98-98: shellcheck reported issue in this script: SC2193:warning:1:34: The arguments to this comparison can never be equal. Make sure your syntax is correct

(shellcheck)

107-107: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

107-107: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

107-107: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

107-107: shellcheck reported issue in this script: SC2086:info:3:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)

.github/workflows/test.yaml (4)

12-19: LGTM! Well-structured setup job

The setup job is well-organized with clear responsibilities and uses the latest action versions.

56-57: Verify the build:test script exists

Ensure that the build:test script is properly defined in package.json.

✅ Verification successful

Build:test script is correctly defined

The build:test script is properly defined in package.json as ember build --environment=test, which is a valid ember build command for test environment.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the build:test script exists in package.json
jq '.scripts["build:test"]' package.json

Length of output: 75

20-37: ⚠️ Potential issue

Add missing setup step

The lint job requires the setup-app action to ensure all dependencies are available.

   lint:
     name: Run Linting
     needs: [setup]
     runs-on: [ubuntu-latest]
     steps:
       - uses: actions/checkout@v4
+      - uses: ./.github/actions/setup-app
       - name: Restore Cached Node Modules
         uses: actions/cache@v4

105-110: 🛠️ Refactor suggestion

Improve Job Summary shell script robustness

The shell script has potential issues with globbing and word splitting.

       - name: Job Summary
         if: always()
         run: |
+          set -eo pipefail
+          {
           echo "| Module | Total | Passed | Failed | Skipped | Duration (ms) |"
           echo "| --- | --- | --- | --- | --- | --- |"
-          cat module-metadata-*.json | jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "' >> $GITHUB_STEP_SUMMARY
+          if ! cat module-metadata-*.json | jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "'; then
+            echo "::error::Failed to process test metadata"
+            exit 1
+          fi
+          } >> "$GITHUB_STEP_SUMMARY"

🧰 Tools

🪛 actionlint (1.7.4)

107-107: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

107-107: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

107-107: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

107-107: shellcheck reported issue in this script: SC2086:info:3:162: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/test.yaml

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (4)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6ff8b9 and c0a1549.

📒 Files selected for processing (6)

.github/actions/setup-app/action.yaml (1 hunks)
.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (5)

app/components/upload-app/status/details/index.ts
package.json
app/components/organization-namespace/request-status/index.ts
.github/actions/setup-app/action.yaml
app/components/organization-namespace/approval-status/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

94-94: shellcheck reported issue in this script: SC2193:warning:1:34: The arguments to this comparison can never be equal. Make sure your syntax is correct

(shellcheck)

103-103: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

103-103: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

103-103: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

103-103: shellcheck reported issue in this script: SC2086:info:3:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (1)

.github/workflows/test.yaml (1)

93-99: ⚠️ Potential issue

Fix shell script comparison syntax

The string comparison in the if condition has incorrect syntax and could fail unexpectedly.

       - name: Run Tests
         run: |
-          if [[ "${{ matrix.tests.type }}" == "Integration Tests - Part"* ]]; then
+          if [[ "${{ matrix.tests.type }}" == "Integration Tests - Part"* || "${{ matrix.tests.type }}" =~ ^Integration\ Tests\ -\ Part ]]; then

🧰 Tools

🪛 actionlint (1.7.4)

94-94: shellcheck reported issue in this script: SC2193:warning:1:34: The arguments to this comparison can never be equal. Make sure your syntax is correct

(shellcheck)

.github/workflows/test.yaml

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c0a1549 and 814ddcc.

📒 Files selected for processing (6)

.github/actions/setup-app/action.yaml (1 hunks)
.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (5)

.github/actions/setup-app/action.yaml
app/components/organization-namespace/request-status/index.ts
package.json
app/components/upload-app/status/details/index.ts
app/components/organization-namespace/approval-status/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

109-109: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

109-109: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

109-109: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

109-109: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/test.yaml

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 814ddcc and fcec6f8.

📒 Files selected for processing (5)

.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

package.json
app/components/upload-app/status/details/index.ts
app/components/organization-namespace/approval-status/index.ts
app/components/organization-namespace/request-status/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (2)

.github/workflows/test.yaml (2)

31-32: ⚠️ Potential issue

Add missing setup step

The lint job requires node_modules but is missing the setup-app action.

       - uses: actions/checkout@v4
+      - name: Setup Application
+        uses: ./.github/actions/setup-app

92-98: 🛠️ Refactor suggestion

Improve job summary shell script robustness

The shell script has issues with globbing, word splitting, and error handling.

       - name: Job Summary
         if: always()
         run: |
+          set -eo pipefail
+          {
           echo "| Module | Total | Passed | Failed | Skipped | Duration (ms) |"
           echo "| --- | --- | --- | --- | --- | --- |"
-          cat module-metadata-*.json | jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "' >> $GITHUB_STEP_SUMMARY
+          if ! find . -name "module-metadata-*.json" -exec cat {} \; | \
+             jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "'; then
+            echo "::error::Failed to process test metadata"
+            exit 1
+          fi
+          } >> "$GITHUB_STEP_SUMMARY"

🧰 Tools

🪛 actionlint (1.7.4)

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/test.yaml

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fcec6f8 and 044bc7e.

📒 Files selected for processing (5)

.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

app/components/organization-namespace/request-status/index.ts
package.json
app/components/organization-namespace/approval-status/index.ts
app/components/upload-app/status/details/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)

.github/workflows/test.yaml (4)

17-18: Fix the build command syntax

The build command should use the proper npm syntax for running Ember commands.

-        run: npm run build:test
+        run: npm run ember -- build --environment=test

26-42: LGTM! Well-structured lint job

The lint job is well configured with proper caching and dependencies.

52-71: Fix incorrect test file paths

The hardcoded paths '/irene/tests/' are incorrect and should use the proper test file patterns.

           - type: Unit Tests
-            commandArgs: --file-path='/irene/tests/unit*.js' --split=3 --parallel=3 --load-balance
+            commandArgs: --file-path='tests/unit/**/*-test.js' --split=3 --parallel=3 --load-balance
           - type: Acceptance Tests
-            commandArgs: --file-path='/irene/tests/acceptance*.js' --split=3 --parallel=3 --load-balance
+            commandArgs: --file-path='tests/acceptance/**/*-test.js' --split=3 --parallel=3 --load-balance
           - type: Integration Tests - Part 1
-            commandArgs: --file-path='/irene/tests/integration*.js' --split=8 --partition=1 --parallel=12 --load-balance
+            commandArgs: --file-path='tests/integration/**/*-test.js' --split=8 --partition=1 --parallel=12 --load-balance

(Apply similar changes to other integration test parts)

88-91: 🛠️ Refactor suggestion

Add error handling for test command

The test command should handle and report failures appropriately.

      - name: Run ${{ matrix.tests.type }}
        run: |
+         set -eo pipefail
          npm run test:ember -- ${{matrix.tests.commandArgs}} --path=dist --wmmf
+         if [ $? -ne 0 ]; then
+           echo "::error::Tests failed for ${{ matrix.tests.type }}"
+           exit 1
+         fi

.github/workflows/test.yaml

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 044bc7e and 899c4cd.

📒 Files selected for processing (1)

.github/workflows/test.yaml (1 hunks)

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)

.github/workflows/test.yaml (4)

17-18: Fix the build command syntax

This issue was previously identified. The build command should use the proper npm syntax for running Ember commands.

26-42: LGTM! Well-structured lint job configuration

The lint job is properly configured with:

Correct dependency on setup job
Efficient caching of node_modules
Simple and clear lint command

88-90: 🛠️ Refactor suggestion

Add error handling to test execution

The test execution step should handle errors properly and provide clear failure messages.

      - name: Run ${{ matrix.tests.type }}
        run: |
+         set -eo pipefail
          npm run test:ember -- ${{matrix.tests.commandArgs}} --path=dist --wmmf
+         if [ $? -ne 0 ]; then
+           echo "::error::Tests failed for ${{ matrix.tests.type }}"
+           exit 1
+         fi

92-98: 🛠️ Refactor suggestion

Improve job summary shell script robustness

The current shell script has issues with globbing and word splitting as identified by shellcheck.

      - name: Job Summary
        if: always()
        run: |
+         set -eo pipefail
+         {
           echo "| Module | Total | Passed | Failed | Skipped | Duration (ms) |"
           echo "| --- | --- | --- | --- | --- | --- |"
-          cat module-metadata-*.json | jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "' >> $GITHUB_STEP_SUMMARY
+          if ! find . -name "module-metadata-*.json" -exec cat {} \; | \
+             jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "'; then
+            echo "::error::Failed to process test metadata"
+            exit 1
+          fi
+         } >> "$GITHUB_STEP_SUMMARY"

🧰 Tools

🪛 actionlint (1.7.4)

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 899c4cd and 4eb9aa7.

📒 Files selected for processing (5)

.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

app/components/organization-namespace/request-status/index.ts
package.json
app/components/upload-app/status/details/index.ts
app/components/organization-namespace/approval-status/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

96-96: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

96-96: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

96-96: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

96-96: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (4)

.github/workflows/test.yaml (4)

26-42: LGTM! Well-structured lint job with proper caching

The lint job is correctly configured with proper dependency on setup and efficient node_modules caching.

90-92: 🛠️ Refactor suggestion

Add error handling for test command

The test command should handle and report failures appropriately.

      - name: Run ${{ matrix.tests.type }}
        run: |
+         set -eo pipefail
          npm run test:ember -- ${{matrix.tests.commandArgs}} --path=dist --wmmf
+         if [ $? -ne 0 ]; then
+           echo "::error::Tests failed for ${{ matrix.tests.type }}"
+           exit 1
+         fi

17-18: ⚠️ Potential issue

Fix the build command syntax

The build command should use the proper npm syntax for running Ember commands.

-        run: npm run build:test
+        run: npm run ember -- build --environment=test

94-100: 🛠️ Refactor suggestion

Improve job summary shell script robustness

The shell script has potential issues with globbing and word splitting.

      - name: Job Summary
        if: always()
        run: |
+         set -eo pipefail
+         {
           echo "| Module | Total | Passed | Failed | Skipped | Duration (ms) |"
           echo "| --- | --- | --- | --- | --- | --- |"
-          cat module-metadata-*.json | jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "' >> $GITHUB_STEP_SUMMARY
+          if ! find . -name "module-metadata-*.json" -exec cat {} \; | \
+             jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "'; then
+            echo "::error::Failed to process test metadata"
+            exit 1
+          fi
+         } >> "$GITHUB_STEP_SUMMARY"

🧰 Tools

🪛 actionlint (1.7.4)

96-96: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

96-96: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

96-96: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

96-96: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/test.yaml

sonarqubecloud · 2024-12-20T14:41:27Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4eb9aa7 and 7b7ae1c.

📒 Files selected for processing (5)

.github/workflows/test.yaml (1 hunks)
app/components/organization-namespace/approval-status/index.ts (1 hunks)
app/components/organization-namespace/request-status/index.ts (1 hunks)
app/components/upload-app/status/details/index.ts (1 hunks)
package.json (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (4)

app/components/organization-namespace/request-status/index.ts
package.json
app/components/upload-app/status/details/index.ts
app/components/organization-namespace/approval-status/index.ts

🧰 Additional context used

🪛 actionlint (1.7.4)

.github/workflows/test.yaml

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

🔇 Additional comments (3)

.github/workflows/test.yaml (3)

17-18: Fix the build command syntax

The build command needs to use the proper npm syntax for running Ember commands.

52-71: Fix incorrect test file paths

The hardcoded paths '/irene/tests/' are incorrect and should use the proper test file patterns.

92-98: 🛠️ Refactor suggestion

Improve job summary shell script robustness

The shell script needs improvements for better error handling and safety.

       - name: Job Summary
         if: always()
         run: |
+          set -eo pipefail
+          {
           echo "| Module | Total | Passed | Failed | Skipped | Duration (ms) |"
           echo "| --- | --- | --- | --- | --- | --- |"
-          cat module-metadata-*.json | jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "' >> $GITHUB_STEP_SUMMARY
+          if ! find . -name "module-metadata-*.json" -exec cat {} \; | \
+             jq -r '.modules[] | "\(.moduleName | gsub("\\|"; "\\\\|")) | \(.total) | \(.passed) | \(.failed) | \(.skipped) | \(.duration) "'; then
+            echo "::error::Failed to process test metadata"
+            exit 1
+          fi
+          } >> "$GITHUB_STEP_SUMMARY"

🧰 Tools

🪛 actionlint (1.7.4)

94-94: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:1:74: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:2:49: Double quote to prevent globbing and word splitting

(shellcheck)

94-94: shellcheck reported issue in this script: SC2086:info:4:162: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/test.yaml

coderabbitai bot reviewed Dec 12, 2024

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 12:21 View deployment

avzz-19 force-pushed the test-partitioning branch from f09b02b to 65ed709 Compare December 12, 2024 12:23

This comment was marked as outdated.

Sign in to view

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 12:26 View deployment

avzz-19 force-pushed the test-partitioning branch from 65ed709 to d5bc824 Compare December 12, 2024 12:28

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 12:31 View deployment

avzz-19 force-pushed the test-partitioning branch from d5bc824 to fd97f03 Compare December 12, 2024 12:55

This comment was marked as outdated.

Sign in to view

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 12:58 View deployment

avzz-19 force-pushed the test-partitioning branch from fd97f03 to 10a7808 Compare December 12, 2024 12:59

This comment was marked as outdated.

Sign in to view

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 13:02 View deployment

avzz-19 force-pushed the test-partitioning branch from 10a7808 to c94b916 Compare December 12, 2024 13:06

appknox deleted a comment from coderabbitai bot Dec 12, 2024

avzz-19 force-pushed the test-partitioning branch 2 times, most recently from 517d76d to c067769 Compare December 12, 2024 13:20

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 13:23 View deployment

avzz-19 force-pushed the test-partitioning branch from c067769 to 55f9f51 Compare December 12, 2024 13:30

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 13:32 View deployment

avzz-19 force-pushed the test-partitioning branch from 55f9f51 to ed683f9 Compare December 12, 2024 14:59

This comment was marked as outdated.

Sign in to view

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 15:02 View deployment

avzz-19 force-pushed the test-partitioning branch 2 times, most recently from 86ece20 to ba215fa Compare December 12, 2024 15:25

This comment was marked as outdated.

Sign in to view

github-actions bot deployed to irenestorybook (Preview) December 12, 2024 15:28 View deployment

avzz-19 force-pushed the test-partitioning branch from ba215fa to 20e524b Compare December 12, 2024 15:34

avzz-19 force-pushed the test-partitioning branch from fb57a9d to c6ff8b9 Compare December 20, 2024 09:59

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 10:02 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

.github/workflows/test.yaml Outdated Show resolved Hide resolved

avzz-19 force-pushed the test-partitioning branch from c6ff8b9 to c0a1549 Compare December 20, 2024 10:09

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 10:12 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

avzz-19 force-pushed the test-partitioning branch from c0a1549 to 6f6ee85 Compare December 20, 2024 10:15

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 10:17 View deployment

avzz-19 force-pushed the test-partitioning branch from 6f6ee85 to 814ddcc Compare December 20, 2024 12:37

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 12:40 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

future-pirate-king force-pushed the test-partitioning branch from 814ddcc to fcec6f8 Compare December 20, 2024 12:57

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 12:59 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Show resolved Hide resolved

.github/workflows/test.yaml Show resolved Hide resolved

future-pirate-king force-pushed the test-partitioning branch from fcec6f8 to 044bc7e Compare December 20, 2024 13:14

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 13:17 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Show resolved Hide resolved

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 13:50 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

avzz-19 force-pushed the test-partitioning branch from 899c4cd to 4eb9aa7 Compare December 20, 2024 14:33

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 14:36 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

lint & test ci workflow parllelization

7b7ae1c

avzz-19 force-pushed the test-partitioning branch from 4eb9aa7 to 7b7ae1c Compare December 20, 2024 14:40

github-actions bot deployed to irenestorybook (Preview) December 20, 2024 14:43 View deployment

coderabbitai bot reviewed Dec 20, 2024

View reviewed changes

.github/workflows/test.yaml Show resolved Hide resolved

future-pirate-king approved these changes Dec 20, 2024

View reviewed changes

future-pirate-king merged commit ec3bd80 into develop Dec 20, 2024
19 checks passed

future-pirate-king deleted the test-partitioning branch December 20, 2024 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ember exam test partitioning #1505

ember exam test partitioning #1505

avzz-19 commented Dec 12, 2024

coderabbitai bot left a comment

cloudflare-workers-and-pages bot commented Dec 12, 2024 •

edited

Loading

This comment was marked as outdated.

cypress bot commented Dec 12, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

sonarqubecloud bot commented Dec 20, 2024

coderabbitai bot left a comment

ember exam test partitioning #1505

ember exam test partitioning #1505

Conversation

avzz-19 commented Dec 12, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Dec 12, 2024 • edited Loading

Deploying irenestaging with Cloudflare Pages

This comment was marked as outdated.

cypress bot commented Dec 12, 2024

Irene Run #543

Tests for review

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 20, 2024

Quality Gate passed

coderabbitai bot left a comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Dec 12, 2024 •

edited

Loading