-
Notifications
You must be signed in to change notification settings - Fork 941
Fix minor bugs in OIDC conformance tests #26518
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThree files are modified to adjust workflow automation and notification formatting. The FAPI OIDC workflow has indentation changes, the OIDC workflow updates tag input descriptions and introduces INPUT_TAG-based RESOURCE derivation for notifications, and the chat notification script adjusts HTML formatting. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
oidc-conformance-tests/send_chat.py (1)
85-85: HTML formatting fix looks correct.The addition of
</font></b></br>\nproperly closes the tags opened on Line 84 and adds a line break before the "Failed test cases" text, which aligns with the PR's goal to improve message formatting.♻️ Optional: Simplify f-string conversions
The explicit
str()calls in f-strings on lines 85-86 are unnecessary since f-strings automatically convert values to strings:- f"</font></b></br>\nFailed test cases: {str(failed_count)}" - f"\nTest cases with errors: {str(errors_count)}" + f"</font></b></br>\nFailed test cases: {failed_count}" + f"\nTest cases with errors: {errors_count}".github/workflows/oidc-conformance-test.yml (1)
267-272: LGTM! INPUT_TAG-based RESOURCE derivation is correct.The logic properly derives the RESOURCE value for chat notifications. Note that the chat context uses "built from latest source" (with spaces) while the email context uses "built-from-latest-source" (with hyphens). This difference may be intentional for display formatting in different notification contexts.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/fapi-oidc-conformance-test.yml.github/workflows/oidc-conformance-test.ymloidc-conformance-tests/send_chat.py
🧰 Additional context used
🪛 Ruff (0.14.10)
oidc-conformance-tests/send_chat.py
85-85: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (3)
.github/workflows/fapi-oidc-conformance-test.yml (1)
321-325: LGTM! Indentation improvements enhance readability.The indentation adjustments improve code readability without altering the logic. The RESOURCE derivation correctly defaults to "built from latest source" when no tag is provided.
.github/workflows/oidc-conformance-test.yml (2)
17-17: LGTM! Clearer input description.The updated description clearly specifies the expected format and fallback behavior, improving user experience when manually triggering the workflow.
242-247: LGTM! INPUT_TAG-based RESOURCE derivation is correct.The logic properly derives the RESOURCE value for email notifications, defaulting to "built-from-latest-source" when no tag is provided. This aligns with the PR's objective to use the
taginput exclusively.
This pull request updates the OIDC conformance test workflow to simplify input handling and improve messaging formatting. The main changes focus on removing the unused
download-urlinput, updating logic to consistently use thetaginput, and refining the chat message output.Workflow input and logic simplification:
download-urlinput from the workflow and updated descriptions for thetaginput to clarify its usage.tagexclusively, defaulting to "built-from-latest-source" or "built from latest source" when not provided, and eliminated references todownload-url. [1] [2]Messaging improvements:
send_chat.pyby ensuring proper HTML line breaks and bold/colored status text.Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.