fix(KFLUXUI-1032): do not remove app when periodic stage e2e test fail#717
fix(KFLUXUI-1032): do not remove app when periodic stage e2e test fail#717
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Test Configuration Logic e2e-tests/tests/basic-happy-path.spec.ts |
Modified the skip condition for the "Delete the application via UI" setup from checking strict string equality (=== 'true') to truthiness evaluation (Cypress.env('PERIODIC_RUN_STAGE')), changing behavior to skip deletion whenever the environment variable is set to any truthy value. |
Estimated code review effort
🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
- test(KFLUXUI-324): flaky test app removal #607: Modifies the same "Delete the application via UI" flow in the basic-happy-path test file with related cleanup/deletion changes.
- fix: periodic jobs config #576: Also changes strict
'true'string checks to truthiness-based evaluation of thePERIODIC_RUN_STAGEenvironment flag. - fix(KFLUXUI-1032): keep application if it fails on stage #699: Modifies deletion-skip logic in the same test file and relies on
PERIODIC_RUN_STAGEconditional logic.
Suggested reviewers
- Katka92
- testcara
- rrosatti
- milantaky
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately describes the main fix: preventing app removal when periodic stage e2e tests fail, which matches the PR's core objective and the code change. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
KFLUXUI-1032
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 @coderabbitai help to get the list of available commands and usage tips.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
6cf1302 to
0230a80
Compare
Keep application and its artifact in case of failure on stage check Jira KFLUXUI-1032 Assisted-by: Cursor
c5bdff4 to
b1fccb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-tests/tests/basic-happy-path.spec.ts`:
- Line 222: The conditional in the test cleanup uses a truthy check on
Cypress.env('PERIODIC_RUN_STAGE') which misinterprets the string 'false' as
true; update the condition in the block that checks hasTestFailed and
Cypress.env('PERIODIC_RUN_STAGE') (the line with if (hasTestFailed &&
Cypress.env('PERIODIC_RUN_STAGE')) {) to perform an explicit string comparison
against 'true' (i.e., check Cypress.env('PERIODIC_RUN_STAGE') === 'true') so
deletion only runs when the env explicitly equals 'true'.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #717 +/- ##
==========================================
- Coverage 87.23% 87.14% -0.10%
==========================================
Files 764 764
Lines 58225 58225
Branches 5658 6879 +1221
==========================================
- Hits 50795 50741 -54
+ Misses 7376 7359 -17
- Partials 54 125 +71
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Assisted-by: Cursor
Fixes
KFLUXUI-1032
Description
Do not remove konflux ui app when periodic e2e stage test fails. Currently we chekcing for
PERIODIC_RUN_STAGEvariable from cypress.config.ts but on the periodic stage test it is set the variableJOB_TYPEthat we can use to check what kind of test is running.Type of change
Screen shots / Gifs for design review
How to test or reproduce?
Browser conformance:
Summary by CodeRabbit