chore(ci): AB-2168 - manual CI workflow trigger, TestRail mapping improvements, and new test case IDs#795
chore(ci): AB-2168 - manual CI workflow trigger, TestRail mapping improvements, and new test case IDs#795sievdokymov-virtru wants to merge 7 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds a manual Changes
Sequence Diagram(s)sequenceDiagram
autonumber
rect rgba(200,230,255,0.5)
actor User as GitHub UI
participant Actions as GitHub Actions CI
participant E2E as opentdf/otdfctl/e2e
participant Script as upload-bats-test-results-to-testrail.sh
participant TestRail as TestRail API
end
User->>Actions: Trigger push or workflow_dispatch (optional testrail-run-name)
Actions->>E2E: Run e2e job (otdfctl-ref -> PR head sha or github.sha)
E2E->>Script: Produce test results & mapping-report.txt
alt on main branch or workflow_dispatch
Script->>TestRail: Upload mapping-report / test results
TestRail-->>Script: Return upload status
else other branches
Script-->>Actions: Skip TestRail upload
end
Script-->>Actions: Persist mapping-report artifact
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request restricts TestRail integration and report uploading to the main branch or manual workflow triggers. It also updates the test case mapping JSON with new entries and modifies the integration script to handle specific test name prefixes and include colored console output. Feedback was provided to ensure that ANSI color codes are not written to the report file artifact, as they can cause readability issues in standard text editors and browser previews.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
e2e/action.yaml (1)
82-97:⚠️ Potential issue | 🟠 MajorAdd
always()to TestRail reporting steps to capture failed test results.Lines 83 and 97 gate by branch/event but lack
always(), so these steps skip afterRun Bats testsfails. This means failed Bats results onmainor manual runs are written tobats-results.tapbut never uploaded to TestRail or included in the mapping-report artifact.Modify the conditions to run regardless of prior test failures:
Proposed fix
- name: Integrate Bats test results into TestRail - if: github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch' + if: always() && (github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch') shell: bash working-directory: otdfctl run: | cd e2e cp ./testrail-integration/samples-for-virtru-instance/testrail-virtru.config.json ./testrail-integration/testrail.config.json cp ./testrail-integration/samples-for-virtru-instance/testname-to-testrail-id.virtru.json ./testrail-integration/testname-to-testrail-id.json ./testrail-integration/upload-bats-test-results-to-testrail.sh continue-on-error: true env: TESTRAIL_USER: ${{ env.TESTRAIL_USER }} TESTRAIL_PASS: ${{ env.TESTRAIL_PASS }} TESTRAIL_CLI_RUN_NAME: ${{ inputs.testrail-run-name-for-cli-test }} - name: Upload TestRail mapping report - if: github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch' + if: always() && (github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch') uses: actions/upload-artifact@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/action.yaml` around lines 82 - 97, The two workflow steps "Integrate Bats test results into TestRail" and "Upload TestRail mapping report" are currently gated by branch/event and will be skipped if earlier steps fail; change their if conditions to include always() so they run regardless of prior failures, e.g. update the if expressions for those steps to always() && (github.ref == 'refs/heads/main' || github.event_name == 'workflow_dispatch') so that the Bats TAP output is always uploaded to TestRail and included in the mapping-report artifact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/testrail-integration/upload-bats-test-results-to-testrail.sh`:
- Around line 139-144: Replace usages of echo -e that interpolate
user-controlled $name (and other variables) with printf to avoid interpreting
backslash sequences in test names; for each occurrence (the two echo -e lines
that print to console and the two that append to "$REPORT_FILE"), use printf
with a %b for the color sequence portion and %s for the $name/test data so only
the color codes are expanded and user input is printed verbatim; ensure the
messages that include $case_id/$section remain formatted the same and keep
appends to "$REPORT_FILE" using printf to avoid corrupting logs.
---
Outside diff comments:
In `@e2e/action.yaml`:
- Around line 82-97: The two workflow steps "Integrate Bats test results into
TestRail" and "Upload TestRail mapping report" are currently gated by
branch/event and will be skipped if earlier steps fail; change their if
conditions to include always() so they run regardless of prior failures, e.g.
update the if expressions for those steps to always() && (github.ref ==
'refs/heads/main' || github.event_name == 'workflow_dispatch') so that the Bats
TAP output is always uploaded to TestRail and included in the mapping-report
artifact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5868b528-6276-4eb2-863e-f43b5bceabff
📒 Files selected for processing (4)
.github/workflows/ci.yamle2e/action.yamle2e/testrail-integration/samples-for-virtru-instance/testname-to-testrail-id.virtru.jsone2e/testrail-integration/upload-bats-test-results-to-testrail.sh
X-Test Failure Report |
|
Fix PR for blocking govulncheck: #796 |
X-Test Failure Report |
|
Closing in favour of opentdf/platform#3361. |
Summary
workflow_dispatchinput toci.yamlso the E2E suite can be triggered manually with a custom TestRail run name, not only on push/schedule.otdfctl-reffor manual runs: Changedgithub.event.pull_request.head.shatogithub.event.pull_request.head.sha || github.shaso the ref resolves correctly when there is no pull request context (e.g. manual dispatch or schedule).action.yamlnow only run onmainbranch orworkflow_dispatch, preventing noise from PR runs.upload-bats-test-results-to-testrail.shnow strips[Auto]/(Auto)prefixes from mapping file keys before comparison, so TestRail case names can carry those prefixes without breaking the TAP→case-ID lookup.YES_MAPPING_FOUND(green) /MAPPING_NOT_FOUND(red) for easier visual scanning of the report.testname-to-testrail-id.virtru.jsonupdated with new case IDs for policy namespaces, obligation triggers, SCS namespace filters, and subject mapping namespace variants.Test plan
[Auto]/(Auto)prefixes in the mapping file are correctly matched against plain TAP names.🤖 Generated with Claude Code
Summary by CodeRabbit