Skip to content

fix(docs): Skip Fern bash-script tests on Windows#2017

Merged
tgasser-nv merged 1 commit into
developfrom
fix/windows-fern-bash-script
Jun 12, 2026
Merged

fix(docs): Skip Fern bash-script tests on Windows#2017
tgasser-nv merged 1 commit into
developfrom
fix/windows-fern-bash-script

Conversation

@tgasser-nv

@tgasser-nv tgasser-nv commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Description

The Full Tests which check both Windows and Linux only run nightly. The unrelated PR #2016 had failing Windows tests on this run. The errors are all in Fern and doc-related changes, introduced in #2015 . This PR wasn't blocked because the Full-Tests weren't run prior to merge.

The 5 test failures all stem from the same issue:

test_reports_broken_local_markdown_links_with_source_line_numbers
test_ignores_links_inside_inline_code_and_html_comments
test_resolves_guardrails_fern_routes
test_rejects_mdx_suffixes_for_links_that_resolve_as_fern_routes
test_fails_loudly_on_malformed_html_comments

All are calling bash to run check-docs-links.sh, which fails because WSL is not available on the Windows runner.

This PR is a temporary fix by removing these tests when running on Windows. However, our users with a Windows laptop would reasonably expect everything to work the same as on MacOS and Linux and this needs to be fixed for them.

Related Issue(s)

#2015 : The PR introducing the regression
#2017 : This PR temporarily disabling tests/test_docs_links.py under Windows .

Checklist

  • I've read the CONTRIBUTING guidelines.
  • I've updated the documentation if applicable.
  • I've added tests if applicable.
  • @mentions of the person or team responsible for reviewing proposed changes.

Summary by CodeRabbit

  • Tests
    • Improved test compatibility on Windows by skipping docs link-checking tests when the required bash-based tool is unavailable on that platform.

@tgasser-nv tgasser-nv self-assigned this Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds Windows-specific test skipping to the docs link checker test suite. The platform and pytest modules are imported, then five test functions that depend on bash scripts are decorated with @pytest.mark.skipif to skip execution on Windows systems.

Changes

Conditional test skip on Windows

Layer / File(s) Summary
Import platform and apply skip markers to bash-dependent tests
tests/test_docs_links.py
Imports platform and pytest, then decorates five bash-dependent tests with @pytest.mark.skipif(platform.system() == "Windows", reason="bash script not available on Windows") to prevent execution on Windows where the link-checking bash script is unavailable.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Test Results For Major Changes ✅ Passed Changes are minor: a platform compatibility fix adding pytest.mark.skipif decorators to skip bash-script-dependent tests on Windows. Per check instructions, passes when changes are minor even witho...
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: skipping Windows-incompatible bash-script tests for Fern documentation.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-fern-bash-script

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

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds @pytest.mark.skipif decorators to all five bash-backed doc-link tests to prevent failures on Windows where bash is unavailable, and pulls in the platform and pytest imports needed.

  • Skips five tests in tests/test_docs_links.py using platform.system() == \"Windows\" so CI on Windows no longer errors on the bash-dependent check-docs-links.sh invocations.
  • The guard criterion could be made more portable by checking shutil.which(\"bash\") is None instead, which would also skip correctly on any system (e.g. a minimal Linux container) missing bash, while still running on Windows systems that do have bash (Git Bash, WSL).

Confidence Score: 4/5

Safe to merge — the change only adds skip guards to test functions and introduces no risk to production code.

All five bash-backed tests now get a skip decorator that prevents false failures on Windows CI. The only concern is that platform.system() == 'Windows' is a weaker proxy for 'bash is unavailable' than shutil.which('bash') is None, which is already imported, but this is a minor quality issue that does not affect correctness on the primary CI targets.

No files require special attention beyond the minor portability note on tests/test_docs_links.py.

Important Files Changed

Filename Overview
tests/test_docs_links.py Adds @pytest.mark.skipif(platform.system() == 'Windows', ...) to all five bash-backed tests; import platform and import pytest added. The guard works but shutil.which('bash') is None would be more portable.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["pytest collect test_docs_links.py"] --> B{"platform.system() == 'Windows'?"}
    B -- Yes --> C["SKIP test\n(all 5 test functions)"]
    B -- No --> D["run_link_check()\ncalls bash check-docs-links.sh"]
    D --> E["assert returncode / output"]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
tests/test_docs_links.py:44-45
Using `platform.system() == "Windows"` will skip tests even when `bash` is genuinely available on Windows (e.g. Git Bash, WSL), and conversely will not skip on non-Windows systems where `bash` is absent. Since `shutil` is already imported, checking `shutil.which("bash") is None` is a more portable condition and avoids a spurious skip on Windows environments that do have bash. The same replacement applies to all five decorators, and `import platform` can be removed once this is changed everywhere.

```suggestion
@pytest.mark.skipif(shutil.which("bash") is None, reason="bash not found in PATH")
def test_reports_broken_local_markdown_links_with_source_line_numbers(
```

Reviews (1): Last reviewed commit: "Skip fern docs tests using bash scripts ..." | Re-trigger Greptile

Comment thread tests/test_docs_links.py
@tgasser-nv

Copy link
Copy Markdown
Collaborator Author

Manually triggered a "Full Tests" run on this branch here. This confirms the Window Full Tests are clean after this merge.

@tgasser-nv tgasser-nv requested review from Pouyanpi and miyoungc June 11, 2026 15:11
@tgasser-nv tgasser-nv changed the title fix(docs): Skip Fern bash-script tests on Windows" fix(docs): Skip Fern bash-script tests on Windows Jun 11, 2026
@tgasser-nv tgasser-nv merged commit 5d96c4b into develop Jun 12, 2026
42 of 43 checks passed
@tgasser-nv tgasser-nv deleted the fix/windows-fern-bash-script branch June 12, 2026 11:36
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.

2 participants