Skip to content

[6.19.z] docs links test improvements - #20876

Merged
pondrejk merged 2 commits into
SatelliteQE:6.19.zfrom
rmynar:sat-41822-chp619
Mar 3, 2026
Merged

[6.19.z] docs links test improvements#20876
pondrejk merged 2 commits into
SatelliteQE:6.19.zfrom
rmynar:sat-41822-chp619

Conversation

@rmynar

@rmynar rmynar commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

cherrypick of #20797 into 6.19 branch - I am creating this pull request ahead of merging the initial PR in order to trigger PRT tests that cannot pass on the original branch because the documentation for version 6.20(stream) does not yet exist.
Improvements to test_positive_documentation_links:

  1. The test is parametrized, so there's one test per page, which makes it easier to analyze results
  2. There's a module scoped session which keeps user logged in during testing links on each page (saves time)
  3. Explicit redirect to single page in staging is no more needed since also production links lead to single page version (Link to the single-page version of docs RedHatSatellite/foreman_theme_satellite#202)

Summary by Sourcery

Tests:

  • Parametrize the documentation links end-to-end test over individual pages and reuse a module-scoped UI session to speed up execution and improve failure isolation.

@rmynar rmynar self-assigned this Feb 25, 2026
@rmynar rmynar added the No-CherryPick PR doesnt need CherryPick to previous branches label Feb 25, 2026
@rmynar

rmynar commented Feb 25, 2026

Copy link
Copy Markdown
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_documentation_links.py

@Satellite-QE

Copy link
Copy Markdown
Collaborator

PRT Result

Build Number: 14483
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_documentation_links.py --external-logging
Test Result : ================= 26 passed, 86 warnings in 1180.37s (0:19:40) =================

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Feb 25, 2026
@sourcery-ai

sourcery-ai Bot commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Refactors the documentation links E2E test to be page-parametrized, reuse a shared UI session per module, and simplify Non-GA documentation URL handling by removing the explicit html-single redirect logic while preserving link validation behavior.

File-Level Changes

Change Details Files
Parametrize documentation links test per page and share the page list at module scope.
  • Move static list of tested UI pages to a module-level pages variable.
  • Use pytest.mark.parametrize to run test_positive_documentation_links once per page instead of looping inside the test.
tests/foreman/ui/test_documentation_links.py
Introduce a module-scoped UI session fixture to keep the user logged in across page checks.
  • Add a module-scoped pytest fixture that opens module_target_sat.ui_session() once per module and yields the session object.
  • Update test_positive_documentation_links signature to accept module_target_sat and the shared session fixture instead of creating a new session in the test body.
tests/foreman/ui/test_documentation_links.py
Simplify per-page link collection and Non-GA documentation URL handling while retaining link validation.
  • Replace the previous nested defaultdict-based aggregation of all_links and pages_with_broken_links with simple per-test all_links and broken_links lists.
  • Keep logic that rewrites documentation URLs to stage_docs_url for Non-GA Satellite versions, but remove forced html-to-html-single conversion and hostname-based redirect assumptions now that production links also resolve to single-page versions.
  • Adjust test assertion and logging messages to be page-specific, making failures easier to interpret.
tests/foreman/ui/test_documentation_links.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • The session fixture name is quite generic and can be confused with requests.Session or pytest internals; consider renaming it to something more specific like ui_session to make call sites clearer and avoid accidental shadowing.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `session` fixture name is quite generic and can be confused with `requests.Session` or pytest internals; consider renaming it to something more specific like `ui_session` to make call sites clearer and avoid accidental shadowing.

## Individual Comments

### Comment 1
<location path="tests/foreman/ui/test_documentation_links.py" line_range="96-98" />
<code_context>
-                        'https://docs.redhat.com', settings.robottelo.stage_docs_url
-                    )
-                    link = link.replace('html', 'html-single')
-                if requests.get(link, verify=False).status_code != 200:
-                    pages_with_broken_links[page].append(link)
-                    logger.info(f"Following link on {page} page seems broken: \n {link}")
-        assert not pages_with_broken_links, (
-            f"There are Satellite pages with broken documentation links. \n {print(pages_with_broken_links)}"
</code_context>
<issue_to_address>
**suggestion (testing):** Add a timeout to `requests.get` calls to reduce flakiness and hanging tests

This test now performs one `requests.get` per link and page, so if the docs host is slow or unresponsive, the test can hang and become flaky. Please add a bounded timeout (e.g. `timeout=10`) to these calls so the test fails quickly and predictably when docs are unavailable, without changing its functional intent.

Suggested implementation:

```python
                if requests.get(link, verify=False, timeout=10).status_code != 200:

```

If there are other `requests.get` calls in this test file (or related documentation-link tests) that perform similar external checks, you may want to add `timeout=10` to those as well for consistency and to avoid other potential hangs.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tests/foreman/ui/test_documentation_links.py

@adamlazik1 adamlazik1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@pondrejk
pondrejk enabled auto-merge (squash) March 3, 2026 09:01
@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Mar 3, 2026
@pondrejk
pondrejk merged commit 3146055 into SatelliteQE:6.19.z Mar 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No-CherryPick PR doesnt need CherryPick to previous branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants