Revert "CI: Fix flaky no-code verifications, add native GitHub Actions success notification for /test-pr slash command (#283)"#551
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the GitHub Actions workflow file Changes
Possibly related PRs
What do you think about these changes? Do you see any other areas that might benefit from a similar simplification? wdyt? Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/test-pr-command.yml (1)
24-26: Hey there! Quick suggestion about shell safety 🛡️Would you mind adding quotes around the environment variables? This prevents potential issues with word splitting and special characters, wdyt?
- run: echo "run-url=https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" >> $GITHUB_OUTPUT + run: echo "run-url=https://github.com/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" >> "${GITHUB_OUTPUT}"🧰 Tools
🪛 actionlint (1.7.4)
26-26: shellcheck reported issue in this script: SC2086:info:1:85: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/test-pr-command.yml(3 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test-pr-command.yml
26-26: shellcheck reported issue in this script: SC2086:info:1:85: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (2)
.github/workflows/test-pr-command.yml (2)
Line range hint 1-94: Overall changes look great! 👍
The workflow simplification makes sense - removing the artifact steps while maintaining the core functionality. The job dependencies and error handling are well structured.
🧰 Tools
🪛 actionlint (1.7.4)
26-26: shellcheck reported issue in this script: SC2086:info:1:85: Double quote to prevent globbing and word splitting
(shellcheck)
94-94: Quick check about test filtering 🤔
I notice we're only excluding "super_slow" tests now. The previous version also excluded "flaky" tests - was removing this filter intentional? Just want to make sure we're not accidentally including potentially unstable tests.
Let's check if we have any flaky tests that might affect the stability:
✅ Verification successful
Heads up: The flaky tests look intentionally marked that way 👀
Looking at the tests marked as flaky, they're specifically for low-code connectors that are expected to change frequently (e.g., when connectors evolve from low-code to no-code). The reruns=0 parameter suggests these are more like "known to be volatile" rather than "unstable" tests. Removing the flaky filter seems reasonable since these tests are meant to fail when connector configurations change.
Would you agree it's safe to proceed without the flaky filter? The failures here would actually be useful signals about connector evolution rather than test instability. wdyt?
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for tests marked as flaky
rg -l "@pytest.mark.flaky"
Length of output: 77
Script:
#!/bin/bash
# Let's check the actual flaky test implementations to understand their nature
rg "@pytest.mark.flaky" -B 2 -A 5
Length of output: 1492
This partially reverts commit cc716f7.
Summary by CodeRabbit