Skip to content

[change] Handle skipped tests in SeleniumTestMixin#570

Merged
nemesifier merged 1 commit intoopenwisp:masterfrom
asmodehn:patch-1
Jan 29, 2026
Merged

[change] Handle skipped tests in SeleniumTestMixin#570
nemesifier merged 1 commit intoopenwisp:masterfrom
asmodehn:patch-1

Conversation

@asmodehn
Copy link
Copy Markdown
Member

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue

Closes #569 .

Description of Changes

Here we check if the test has been skipped, to correctly report it as skipped.
It s a quick patch that worked on my setup. Feel free to adjust.

Related concerns:

We might want to double check the behavior of an @expectedFailure test marker...

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jan 29, 2026

📝 Walkthrough

Walkthrough

Adds skip propagation to the SeleniumTestMixin retry logic so that if a retry attempt yields a skipped test, each skip reason is forwarded to the original test result via addSkip and retrying stops. The propagation is applied both after the initial super()._setup_and_call call and inside the per-attempt retry loop, ensuring skipped outcomes are reported as skipped rather than counted as successes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: handling skipped tests in SeleniumTestMixin, which matches the PR's core objective of fixing how skipped tests are reported.
Description check ✅ Passed The description covers key sections: checklist items (contributing guidelines and manual testing completed), reference to linked issue #569, and explanation of changes. However, it lacks detail about the actual implementation and notes that new tests and documentation weren't updated.
Linked Issues check ✅ Passed The code changes address issue #569 by propagating skipped tests during retries in SeleniumTestMixin, ensuring skipped tests are correctly reported as skipped rather than as successes.
Out of Scope Changes check ✅ Passed All changes in openwisp_utils/tests/selenium.py are directly scoped to addressing the skip propagation issue identified in issue #569; no out-of-scope modifications were introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5d2c63 and d988fee.

📒 Files selected for processing (1)
  • openwisp_utils/tests/selenium.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • openwisp_utils/tests/selenium.py

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jan 29, 2026
Copy link
Copy Markdown
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @asmodehn, LGTM.

Let's avoid unnecessary blank lines because it's making my reviewer life harder, I'll commit these suggestions unless this stupid github UI gives me error again.

Comment thread openwisp_utils/tests/selenium.py Outdated
Comment thread openwisp_utils/tests/selenium.py Outdated
@nemesifier
Copy link
Copy Markdown
Member

nemesifier commented Jan 29, 2026

I can't apply the suggestions myself unfortunately:

Screenshot from 2026-01-29 12-05-01

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 97.115% (-0.1%) from 97.255%
when pulling d988fee on asmodehn:patch-1
into 195a1ec on openwisp:master.

@asmodehn asmodehn requested a review from nemesifier January 29, 2026 16:30
@nemesifier nemesifier merged commit 1caa9ce into openwisp:master Jan 29, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] SeleniumTestMixin incorrectly report skipped tests as success

3 participants