Unicron v3 apis test coverage dev 4#4849
Conversation
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
Signed-off-by: Lukasz Gryglicki <lgryglicki@cncf.io> Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot)
lukaszgryglicki
requested review from
ahmedomosanya,
Copilot,
jarias-lfx and
mlehotskylf
WalkthroughThis PR adds comprehensive API testing infrastructure for V3 and V4 endpoints, including Cypress E2E tests with response logging and JSON schema validation, new fixture files for response validation, and Bash utility scripts for endpoint testing with authentication and curl execution handling. Changes
Sequence DiagramsequenceDiagram
participant Test as Cypress Test
participant API as API Endpoint
participant Logger as cy.logJson()
participant Validator as validateApiResponse()
participant Assert as Assertions
Test->>API: cy.request(...)
API-->>Test: response
Test->>Logger: cy.logJson('response', response)
activate Logger
Note over Logger: Log response to debug output
Logger->>Test: then(() => { ... })
Test->>Validator: validateApiResponse(fixture, response)
activate Validator
Note over Validator: Match response against JSON schema
Validator-->>Test: validation result
deactivate Validator
Test->>Assert: cy.wrap(...).should(...)
activate Assert
Note over Assert: Verify status codes and properties
Assert-->>Test: pass/fail
deactivate Assert
deactivate Logger
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas requiring extra attention:
Possibly Related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Nitpick comments (27)
utils/v3/shared/handle_api_url.sh (1)
6-14: LGTM! Consider documenting REMOTE values.The URL switching logic is correct and straightforward. The script properly exports API_URL for child processes.
Optionally, you could add a comment documenting the REMOTE environment variable:
+# REMOTE environment variable controls the target: +# REMOTE=1 -> use remote dev server +# otherwise -> use localhost if [ -z "$API_URL" ] then if [ "$REMOTE" = "1" ]utils/v4/shared/handle_api_url.sh (1)
6-14: LGTM! Consider refactoring duplication with V3.The logic is correct and handles URL configuration properly for V4 endpoints.
The logic is nearly identical to
utils/v3/shared/handle_api_url.sh. Consider creating a shared URL handler that accepts the API version as a parameter to reduce duplication:# utils/shared/handle_api_url.sh # Usage: . ./utils/shared/handle_api_url.sh v3|v4 VERSION="${1:-v3}" if [ -z "$API_URL" ]; then if [ "$REMOTE" = "1" ]; then case "$VERSION" in v3) export API_URL="https://api.lfcla.dev.platform.linuxfoundation.org";; v4) export API_URL="https://api-gw.dev.platform.linuxfoundation.org/cla-service";; esac else export API_URL="http://localhost:5001" fi fiThen source it with:
. "$SHARED_DIR/handle_api_url.sh" v4utils/v3/health/get_health.sh (1)
13-16: Static analysis warnings are false positives.The variables
API,CURL_CMD, andUSE_JQare consumed by the sourcedhandle_curl_execution.shscript. Shellcheck cannot detect this pattern, resulting in false positive warnings.To improve clarity and silence warnings, you could add a comment:
# Set up curl execution +# These variables are consumed by handle_curl_execution.sh API="${API_URL}/v3/ops/health" CURL_CMD="curl -s -XGET -H \"Content-Type: application/json\"" USE_JQ=true . ./utils/shared/handle_curl_execution.shOr consider exporting variables if the sourced script expects them:
-API="${API_URL}/v3/ops/health" -CURL_CMD="curl -s -XGET -H \"Content-Type: application/json\"" -USE_JQ=true +export API="${API_URL}/v3/ops/health" +export CURL_CMD="curl -s -XGET -H \"Content-Type: application/json\"" +export USE_JQ=truetests/functional/cypress/e2e/v3/organization.cy.ts (1)
28-28: Consider applying validation to other test cases.Schema validation is only applied to the first test case (line 28). For consistency and comprehensive validation, consider adding
validateApiResponseto the other successful test cases as well (lines 46, 63).Apply validation to other test cases:
# In the second test (around line 46): expect(response.body).to.be.an('object'); expect(response.body.list).to.be.an('array'); expect(response.body.list.length).to.be.greaterThan(0); + validateApiResponse('organization/searchOrganization.json', response); }); }); });# In the third test (around line 63): expect(response.body).to.be.an('object'); expect(response.body.list).to.be.an('array'); expect(response.body.list.length).to.be.greaterThan(0); + validateApiResponse('organization/searchOrganization.json', response); }); }); });tests/functional/cypress/fixtures/health/getHealth.json (1)
1-28: Schema structure looks good. Consider adding format constraints.The schema correctly defines the health response structure with appropriate types and required fields.
For more robust validation, consider adding:
- Format constraints for timestamp fields:
"TimeStamp": { - "type": "string" + "type": "string", + "format": "date-time" },"BuildDate": { - "type": "string" + "type": "string", + "format": "date-time" }
- Strict additional properties handling:
"required": [ "Status" - ] + ], + "additionalProperties": false }This would catch unexpected fields in responses and ensure timestamps follow RFC 3339 format.
utils/v4/version/test_all_version_apis.sh (1)
1-21: LGTM! Consider adding trailing newline.The test orchestration logic is correct and follows the established pattern.
POSIX convention recommends files end with a newline. Consider adding one after line 21.
tests/functional/cypress/fixtures/organization/searchOrganization.json (1)
1-35: Valid JSON Schema structure.The schema appropriately defines organization search results with proper typing and minimal required fields.
Consider adding a trailing newline after line 35 per POSIX convention.
utils/v3/docs/get_swagger_json.sh (1)
13-16: LGTM! Shellcheck warnings are false positives.The variables
API,CURL_CMD, andUSE_JQare consumed by the sourcedhandle_curl_execution.shscript (line 16). This is the established pattern used throughout the test utilities.Consider adding a trailing newline after line 16 per POSIX convention.
utils/v3/health/test_all_health_apis.sh (1)
1-26: LGTM! Test orchestration is well-structured.The script correctly sets up the API URL and executes the health endpoint test.
Consider adding a trailing newline after line 26 per POSIX convention.
tests/functional/cypress/fixtures/users/createUser.json (1)
1-49: Comprehensive user schema.The schema properly defines all user properties with appropriate typing and sensible required fields for user creation.
Consider adding a trailing newline after line 49 per POSIX convention.
utils/v3/users/get_user.sh (1)
17-29: LGTM! Shellcheck warnings are false positives.The variables
API,CURL_CMD, andUSE_JQare consumed by the sourcedhandle_curl_execution.shscript (line 29).Minor observations:
- Mixed path styles: lines 20 and 29 use
./utils/...(repo root relative) while line 23 uses${SCRIPT_DIR}/../...(script relative). Consider using a consistent approach for better maintainability.- Add a trailing newline after line 29 per POSIX convention.
tests/functional/cypress/fixtures/users/searchUsers.json (1)
14-42: Consider adding required fields to user objects.The schema defines numerous properties for user objects (userID, userExternalID, username, etc.) but marks none as required. While this provides flexibility, it may be too permissive and could miss data quality issues in API responses.
Consider adding at least
userIDas a required field for user objects:"type": "object", "properties": { "userID": { "type": "string" }, "userExternalID": { "type": "string" }, ... "githubUsername": { "type": "string" } - } + }, + "required": ["userID"] } } },utils/v4/health/get_health.sh (1)
10-10: Fix inconsistent path sourcing pattern.Line 10 uses a relative path via
${SCRIPT_DIR}/../shared/handle_api_url.sh, while line 16 uses an absolute path./utils/shared/handle_curl_execution.sh. This inconsistency can cause failures depending on the script's invocation context (e.g., when called from different directories).Standardize on using
${SCRIPT_DIR}for all relative sourcing:-. ./utils/shared/handle_curl_execution.sh +. ${SCRIPT_DIR}/../../shared/handle_curl_execution.shAlternatively, if the script is always run from the repository root, document this requirement clearly in the usage comments.
Also applies to: 16-16
utils/v3/docs/get_api_docs.sh (1)
10-10: Fix inconsistent path sourcing pattern.Line 10 uses a relative path via
${SCRIPT_DIR}/../shared/handle_api_url.sh, while line 16 uses an absolute path./utils/shared/handle_curl_execution.sh. This inconsistency can cause failures depending on the script's invocation context.Standardize on using
${SCRIPT_DIR}for all relative sourcing:-. ./utils/shared/handle_curl_execution.sh +. ${SCRIPT_DIR}/../../shared/handle_curl_execution.shAlso applies to: 16-16
utils/v3/users/update_user.sh (1)
48-48: Fix inconsistent path sourcing pattern.Line 22 uses a relative path
./utils/shared/handle_auth.sh, while line 25 uses${SCRIPT_DIR}/../shared/handle_api_url.sh, and line 50 sources./utils/shared/handle_curl_execution.sh. This inconsistency can cause failures depending on where the script is invoked from.Standardize on using
${SCRIPT_DIR}for all sourcing:-# Handle authentication -. ./utils/shared/handle_auth.sh - -# Handle API URL -. ${SCRIPT_DIR}/../shared/handle_api_url.sh +# Handle authentication +. ${SCRIPT_DIR}/../../shared/handle_auth.sh + +# Handle API URL +. ${SCRIPT_DIR}/../shared/handle_api_url.shAnd later:
-. ./utils/shared/handle_curl_execution.sh +. ${SCRIPT_DIR}/../../shared/handle_curl_execution.shutils/v4/version/get_version.sh (1)
10-10: Fix inconsistent path sourcing pattern.Line 10 uses a relative path via
${SCRIPT_DIR}/../shared/handle_api_url.sh, while line 16 uses an absolute path./utils/shared/handle_curl_execution.sh. This inconsistency can cause failures depending on the script's invocation context.Standardize on using
${SCRIPT_DIR}for all relative sourcing:-. ./utils/shared/handle_curl_execution.sh +. ${SCRIPT_DIR}/../../shared/handle_curl_execution.shAlso applies to: 16-16
utils/v3/users/create_user.sh (1)
27-27: Fix inconsistent path sourcing pattern.Line 27 uses
./utils/shared/handle_auth.shwhile line 30 uses${SCRIPT_DIR}/../shared/handle_api_url.sh, and line 52 sources./utils/shared/handle_curl_execution.sh. This inconsistency can cause failures when the script is run from different working directories.Standardize on using
${SCRIPT_DIR}for all sourcing:-# Handle authentication -. ./utils/shared/handle_auth.sh - -# Handle API URL -. ${SCRIPT_DIR}/../shared/handle_api_url.sh +# Handle authentication +. ${SCRIPT_DIR}/../../shared/handle_auth.sh + +# Handle API URL +. ${SCRIPT_DIR}/../shared/handle_api_url.shAnd later:
-. ./utils/shared/handle_curl_execution.sh +. ${SCRIPT_DIR}/../../shared/handle_curl_execution.shutils/v3/users/get_user_by_username.sh (1)
26-29: ShellCheck warnings are false positives; consider adding trailing newline.The variables
API,CURL_CMD, andUSE_JQare consumed by the sourced scripthandle_curl_execution.sh, so ShellCheck's unused-variable warnings can be safely ignored.Optionally, add a trailing newline at the end of the file (after line 29) to follow standard POSIX conventions.
utils/v3/users/search_users.sh (1)
16-39: Consider simplifying query parameter construction.The current manual approach to building query parameters is verbose and repetitive. Consider refactoring to use an array-based approach for better maintainability.
Additionally, prefer
[ -n "$var" ]over[ ! -z "$var" ]as it's more idiomatic in shell scripting.Example refactor:
# Build query parameters -QUERY_PARAMS="" -if [ ! -z "$1" ]; then - QUERY_PARAMS="searchTerm=$1" -fi -if [ ! -z "$2" ]; then - if [ ! -z "$QUERY_PARAMS" ]; then - QUERY_PARAMS="${QUERY_PARAMS}&fullMatch=$2" - else - QUERY_PARAMS="fullMatch=$2" - fi -fi -if [ ! -z "$3" ]; then - if [ ! -z "$QUERY_PARAMS" ]; then - QUERY_PARAMS="${QUERY_PARAMS}&pageSize=$3" - else - QUERY_PARAMS="pageSize=$3" - fi -fi +params=() +[ -n "$1" ] && params+=("searchTerm=$1") +[ -n "$2" ] && params+=("fullMatch=$2") +[ -n "$3" ] && params+=("pageSize=$3") +QUERY_PARAMS=$(IFS='&'; echo "${params[*]}") API="${API_URL}/v3/users/search" -if [ ! -z "$QUERY_PARAMS" ]; then +if [ -n "$QUERY_PARAMS" ]; then API="${API}?${QUERY_PARAMS}" fiutils/v3/users/get_user_compat.sh (1)
23-26: ShellCheck warnings are false positives; consider adding trailing newline.The variables
API,CURL_CMD, andUSE_JQare consumed by the sourcedhandle_curl_execution.shscript, so the unused-variable warnings can be safely ignored.Optionally, add a trailing newline at the end of the file (after line 26) to follow standard POSIX conventions.
utils/v3/organization/search_organization.sh (1)
31-40: Consider proper URL encoding instead of manual sed-based encoding.Lines 31–40 only encode spaces as
%20, which is insufficient for full URL encoding. Query parameters may contain special characters (&,=,#,?, etc.) that need encoding. This can cause malformed API requests.Consider using a dedicated URL-encoding utility or
jq --uri-output-flags, if available, or a small helper function. For minimal change, document this limitation and add a comment.utils/v4/metrics/test_all_metrics_apis.sh (3)
27-27: Strengthen credential validation logic.Line 27 checks
if [ ! -z "$TOKEN" ] && [ ! -z "$XACL" ]to run authenticated tests. This is correct, but the preceding credential reads (lines 19–20, 24–25) suppress errors silently. Add logging to clarify when credentials are missing or empty.if [ -z "$TOKEN" ] then - TOKEN="$(cat ./token.secret 2>/dev/null || echo '')" + TOKEN="$(cat ./token.secret 2>/dev/null || true)" + if [ -z "$TOKEN" ]; then + echo "[INFO] TOKEN not found in environment or ./token.secret file" + fi fi if [ -z "$XACL" ] then - XACL="$(cat ./x-acl.secret 2>/dev/null || echo '')" + XACL="$(cat ./x-acl.secret 2>/dev/null || true)" + if [ -z "$XACL" ]; then + echo "[INFO] XACL not found in environment or ./x-acl.secret file" + fi fi
30-30: Add error handling and status tracking for subshell test invocations.Lines 30, 35, 40, 45, and 50 invoke test scripts without checking their exit status. If a test fails, the orchestrator continues silently. Consider capturing exit codes and reporting failures.
echo "1. Testing GET /metrics/cla-manager-distribution (authenticated)" echo " Command: ${SCRIPT_DIR}/get_cla_manager_distribution.sh" -${SCRIPT_DIR}/get_cla_manager_distribution.sh +if ${SCRIPT_DIR}/get_cla_manager_distribution.sh; then + echo " [PASSED]" +else + echo " [FAILED] Exit code: $?" >&2 +fiRepeat for all 5 API tests.
Also applies to: 35-35, 40-40, 45-45, 50-50
70-70: Fix command hint in credential error message.Line 70 references
$0which will expand to the name of this orchestrator script. This works, but can be clearer by using the full command path or the script name explicitly.- echo " TOKEN=\"\$(cat ./token.secret)\" XACL=\"\$(cat ./x-acl.secret)\" $0" + echo " TOKEN=\"\$(cat ./token.secret)\" XACL=\"\$(cat ./x-acl.secret)\" ${SCRIPT_DIR}/$(basename $0)"utils/v4/health/test_all_health_apis.sh (1)
18-18: Add error handling for subshell test invocation.Line 18 invokes get_health.sh without checking its exit status. If the test fails, execution continues silently.
echo "1. Testing GET /ops/health (public endpoint)" echo " Command: ${SCRIPT_DIR}/get_health.sh" -${SCRIPT_DIR}/get_health.sh +if ${SCRIPT_DIR}/get_health.sh; then + echo "[PASSED]" +else + echo "[FAILED] Exit code: $?" >&2 + exit 1 +fi echo ""utils/v3/test_all_v3_apis.sh (2)
47-47: Add error handling and status tracking for test script invocations.Lines 47, 54, 61, 68, and 76 invoke test scripts without checking their exit status. If a test fails, execution continues and the summary at the end incorrectly reports success.
# Test Documentation APIs (public) echo "=======================================" echo "1. DOCUMENTATION APIs (Public)" echo "=======================================" -${SCRIPT_DIR}/docs/test_all_docs_apis.sh +if ! ${SCRIPT_DIR}/docs/test_all_docs_apis.sh; then + echo "[ERROR] Documentation API tests failed" >&2 + exit 1 +fi echo ""Repeat for all test invocations, or collect exit codes and report at the end.
Also applies to: 54-54, 61-61, 68-68, 76-76
84-84: Improve command hint in credential error message.Line 84 uses
$0in a string context. For clarity, use an absolute or explicit relative path.- echo " TOKEN=\"\$(cat ./token.secret)\" XACL=\"\$(cat ./x-acl.secret)\" $0" + echo " TOKEN=\"\$(cat ./token.secret)\" XACL=\"\$(cat ./x-acl.secret)\" ${SCRIPT_DIR}/$(basename $0)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (52)
tests/functional/cypress/e2e/v3/docs.cy.ts(3 hunks)tests/functional/cypress/e2e/v3/health.cy.ts(3 hunks)tests/functional/cypress/e2e/v3/organization.cy.ts(2 hunks)tests/functional/cypress/e2e/v3/users.cy.ts(9 hunks)tests/functional/cypress/e2e/v3/version.cy.ts(3 hunks)tests/functional/cypress/fixtures/health/getHealth.json(1 hunks)tests/functional/cypress/fixtures/organization/searchOrganization.json(1 hunks)tests/functional/cypress/fixtures/users/createUser.json(1 hunks)tests/functional/cypress/fixtures/users/getUser.json(1 hunks)tests/functional/cypress/fixtures/users/getUserCompat.json(1 hunks)tests/functional/cypress/fixtures/users/searchUsers.json(1 hunks)utils/shared/handle_auth.sh(1 hunks)utils/shared/handle_curl_execution.sh(1 hunks)utils/shared/handle_token.sh(1 hunks)utils/shared/handle_xacl.sh(1 hunks)utils/v3/docs/get_api_docs.sh(1 hunks)utils/v3/docs/get_swagger_json.sh(1 hunks)utils/v3/docs/test_all_docs_apis.sh(1 hunks)utils/v3/health/get_health.sh(1 hunks)utils/v3/health/test_all_health_apis.sh(1 hunks)utils/v3/organization/search_organization.sh(1 hunks)utils/v3/organization/test_all_organization_apis.sh(1 hunks)utils/v3/shared/handle_api_url.sh(1 hunks)utils/v3/test_all_v3_apis.sh(1 hunks)utils/v3/users/create_user.sh(1 hunks)utils/v3/users/delete_user.sh(1 hunks)utils/v3/users/get_user.sh(1 hunks)utils/v3/users/get_user_by_username.sh(1 hunks)utils/v3/users/get_user_compat.sh(1 hunks)utils/v3/users/search_users.sh(1 hunks)utils/v3/users/test_all_users_apis.sh(1 hunks)utils/v3/users/update_user.sh(1 hunks)utils/v3/version/get_version.sh(1 hunks)utils/v3/version/test_all_version_apis.sh(1 hunks)utils/v4/docs/get_api_docs.sh(1 hunks)utils/v4/docs/get_swagger_json.sh(1 hunks)utils/v4/docs/test_all_docs_apis.sh(1 hunks)utils/v4/health/get_health.sh(1 hunks)utils/v4/health/test_all_health_apis.sh(1 hunks)utils/v4/metrics/get_cla_manager_distribution.sh(1 hunks)utils/v4/metrics/get_company_metric.sh(1 hunks)utils/v4/metrics/get_project_metric.sh(1 hunks)utils/v4/metrics/get_top_companies.sh(1 hunks)utils/v4/metrics/get_top_projects.sh(1 hunks)utils/v4/metrics/get_total_count.sh(1 hunks)utils/v4/metrics/list_company_project_metrics.sh(1 hunks)utils/v4/metrics/list_project_metrics.sh(1 hunks)utils/v4/metrics/test_all_metrics_apis.sh(1 hunks)utils/v4/shared/handle_api_url.sh(1 hunks)utils/v4/test_all_v4_apis.sh(1 hunks)utils/v4/version/get_version.sh(1 hunks)utils/v4/version/test_all_version_apis.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/functional/cypress/e2e/v3/docs.cy.ts (1)
tests/functional/cypress/support/commands.js (2)
validate_200_Status(47-52)validate_expected_status(155-198)
tests/functional/cypress/e2e/v3/health.cy.ts (1)
tests/functional/cypress/support/commands.js (3)
validate_200_Status(47-52)validateApiResponse(5-25)validate_expected_status(155-198)
tests/functional/cypress/e2e/v3/users.cy.ts (1)
tests/functional/cypress/support/commands.js (5)
getTokenKey(289-324)bearerToken(288-288)getXACLHeaders(268-280)validate_200_Status(47-52)validateApiResponse(5-25)
tests/functional/cypress/e2e/v3/version.cy.ts (1)
tests/functional/cypress/support/commands.js (2)
validateApiResponse(5-25)validate_expected_status(155-198)
tests/functional/cypress/e2e/v3/organization.cy.ts (1)
tests/functional/cypress/support/commands.js (1)
validateApiResponse(5-25)
🪛 Shellcheck (0.11.0)
utils/v4/metrics/get_project_metric.sh
[warning] 39-39: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 40-40: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/version/get_version.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/get_user_compat.sh
[warning] 23-23: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 24-24: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 25-25: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/update_user.sh
[warning] 47-47: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 48-48: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 49-49: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/organization/search_organization.sh
[warning] 48-48: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 49-49: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/get_top_projects.sh
[warning] 16-16: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 17-17: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/docs/get_swagger_json.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/list_company_project_metrics.sh
[warning] 26-26: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 27-27: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 28-28: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/list_project_metrics.sh
[warning] 36-36: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 37-37: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/docs/get_api_docs.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/get_company_metric.sh
[warning] 26-26: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 27-27: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 28-28: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/get_user_by_username.sh
[warning] 26-26: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 27-27: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 28-28: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/docs/get_api_docs.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/delete_user.sh
[warning] 26-26: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 27-27: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 28-28: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/get_total_count.sh
[warning] 16-16: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 17-17: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/get_user.sh
[warning] 26-26: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 27-27: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 28-28: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/health/get_health.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/health/get_health.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/get_top_companies.sh
[warning] 16-16: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 17-17: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/metrics/get_cla_manager_distribution.sh
[warning] 16-16: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 17-17: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 18-18: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/create_user.sh
[warning] 49-49: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 50-50: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 51-51: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/docs/get_swagger_json.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v4/version/get_version.sh
[warning] 13-13: API appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 14-14: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 15-15: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
utils/v3/users/search_users.sh
[warning] 42-42: CURL_CMD appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 43-43: USE_JQ appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cypress-functional
- GitHub Check: build-test-lint
🔇 Additional comments (21)
utils/shared/handle_auth.sh (1)
1-11: LGTM! Clean authentication wrapper.The script properly resolves its directory and sources the authentication handlers. The usage documentation is clear.
utils/v4/test_all_v4_apis.sh (1)
27-87: Well-structured test orchestrator.The script provides clear output, conditional test execution based on authentication availability, and a helpful summary. The approach of gracefully skipping authenticated tests when credentials are unavailable is user-friendly.
tests/functional/cypress/e2e/v3/organization.cy.ts (1)
4-4: LGTM! Import added correctly.The
validateApiResponseimport is properly added to support schema validation.tests/functional/cypress/e2e/v3/docs.cy.ts (1)
16-19: Consistent response logging enhances test observability.The addition of
cy.logJsonwrapping before validations provides better debugging capabilities while maintaining proper async execution flow.Also applies to: 30-36, 84-93
utils/v3/users/get_user.sh (1)
8-15: Good input validation with clear usage guidance.The parameter validation and error messages are helpful and include a concrete example.
utils/v3/organization/test_all_organization_apis.sh (1)
1-43: LGTM! Well-structured test orchestrator.The script provides clear test organization with descriptive output, proper environment variable handling, and comprehensive coverage of organization search scenarios (by company name, by website, by both, and negative case).
tests/functional/cypress/fixtures/users/getUser.json (1)
1-48: LGTM! Well-structured user schema.The schema appropriately defines a user object with userID as the only required field, which aligns with typical API response validation patterns. All expected user properties are present and properly typed.
tests/functional/cypress/e2e/v3/health.cy.ts (3)
1-6: LGTM! Proper import of validation utility.The addition of
validateApiResponseimport enhances test coverage by enabling schema validation against fixtures.
21-28: LGTM! Proper logging and validation integration.The response handling correctly wraps assertions within
cy.logJson().then()to ensure logging occurs before validation, and adds schema validation viavalidateApiResponse.
65-74: LGTM! Consistent error case handling.The error cases follow the same pattern as the success case, properly logging responses before performing assertions and validations.
tests/functional/cypress/fixtures/users/getUserCompat.json (1)
1-63: LGTM!The JSON Schema is well-formed and properly defines the user compatibility response structure. Nullable fields are correctly typed, and required fields are appropriate for this endpoint.
utils/v4/docs/test_all_docs_apis.sh (1)
1-26: LGTM!The orchestrator script is well-structured with clear output formatting and proper error context. The sequential execution of related endpoints is appropriate for E2E testing.
utils/v3/users/create_user.sh (1)
7-13: Insufficient input validation for required parameters.The validation only checks if parameters exist but doesn't validate their format or content. For example,
githubIDcould be empty after parameter expansion, or contain special characters that break JSON when inserted into the payload.While the parameter count check prevents immediate runtime errors, consider adding format validation for sensitive fields:
# Example validation if ! [[ $userExternalID =~ ^[a-zA-Z0-9_-]+$ ]]; then echo "Error: userExternalID contains invalid characters" exit 1 fiHowever, the more critical issue is in the payload construction (see next comment).
utils/v3/docs/test_all_docs_apis.sh (1)
1-34: LGTM! Clean test orchestrator implementation.The script follows best practices with clear usage examples, proper directory resolution, and appropriate output truncation for readability.
utils/v3/version/test_all_version_apis.sh (1)
1-26: LGTM! Consistent test orchestration pattern.The script follows the established V3 test orchestration pattern with proper setup and clear output formatting.
utils/v3/users/search_users.sh (1)
42-44: ShellCheck warnings are false positives.The variables
CURL_CMDandUSE_JQare consumed by the sourced scripthandle_curl_execution.sh, so the unused-variable warnings can be safely ignored.utils/v4/metrics/get_top_companies.sh (1)
1-19: LGTM! ShellCheck warnings are false positives.The script follows the established pattern for authenticated API calls. The variables flagged by ShellCheck (
API,CURL_CMD,USE_JQ) are consumed by the sourcedhandle_curl_execution.shscript.tests/functional/cypress/e2e/v3/version.cy.ts (3)
1-6: LGTM! Appropriate import addition.Adding
validateApiResponseto the imports aligns with the PR's pattern of adding JSON schema validation across V3 tests.
26-26: LGTM! Schema validation enhances test coverage.The addition of
validateApiResponseensures the version endpoint response conforms to the expected JSON schema, improving test reliability.
65-74: LGTM! Improved logging for failure cases.Wrapping the validation logic with
cy.logJsonimproves observability and debugging for expected failure scenarios, consistent with patterns across other V3 tests in this PR.utils/v4/metrics/get_cla_manager_distribution.sh (1)
1-19: LGTM! ShellCheck warnings are false positives.The script follows the established V4 metrics pattern. The variables flagged by ShellCheck (
API,CURL_CMD,USE_JQ) are consumed by the sourcedhandle_curl_execution.shscript.
There was a problem hiding this comment.
Pull Request Overview
This pull request adds comprehensive testing infrastructure and test coverage for V3 and V4 API endpoints. It includes shell scripts for manual API testing and Cypress test enhancements with JSON schema validation.
- Added shell scripts for testing V3 and V4 API endpoints (docs, health, version, users, organization, metrics)
- Introduced shared utility scripts for authentication, API URL handling, and curl execution
- Enhanced Cypress tests with JSON schema validation and improved error handling
- Added comprehensive CRUD test coverage for V3 users API with proper cleanup
Reviewed Changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/v4/version/test_all_version_apis.sh | V4 version API test orchestration script |
| utils/v4/version/get_version.sh | V4 version endpoint test script |
| utils/v4/test_all_v4_apis.sh | Main V4 API test orchestration script |
| utils/v4/shared/handle_api_url.sh | Shared V4 API URL configuration handler |
| utils/v4/metrics/*.sh | V4 metrics API test scripts (8 files) |
| utils/v4/health/*.sh | V4 health API test scripts (2 files) |
| utils/v4/docs/*.sh | V4 documentation API test scripts (3 files) |
| utils/v3/version/*.sh | V3 version API test scripts (2 files) |
| utils/v3/users/*.sh | V3 users API test scripts (7 files) |
| utils/v3/test_all_v3_apis.sh | Main V3 API test orchestration script |
| utils/v3/shared/handle_api_url.sh | Shared V3 API URL configuration handler |
| utils/v3/organization/*.sh | V3 organization API test scripts (2 files) |
| utils/v3/health/*.sh | V3 health API test scripts (2 files) |
| utils/v3/docs/*.sh | V3 documentation API test scripts (3 files) |
| utils/shared/*.sh | Shared authentication and curl utilities (4 files) |
| tests/functional/cypress/fixtures/**/*.json | JSON schemas for API response validation (6 files) |
| tests/functional/cypress/e2e/v3/*.cy.ts | Enhanced Cypress tests for V3 APIs (5 files) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
V3 E2E test coverage and testing script utils
cc @mlehotskylf @ahmedomosanya @jarias-lfx
Signed-off-by: Lukasz Gryglicki lgryglicki@cncf.io
Assisted by OpenAI
Assisted by GitHub Copilot