Skip to content

test: make the test suite path handling windows-safe - #607

Merged
johnfercher merged 1 commit into
johnfercher:masterfrom
AmmrFX:fix/windows-test-paths
Sep 10, 2026
Merged

johnfercher merged 1 commit into
johnfercher:masterfrom
AmmrFX:fix/windows-test-paths

Conversation

@AmmrFX

@AmmrFX AmmrFX commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description

Makes the test suite runnable on Windows. go test ./... currently fails there in three ways, all from path constructions that assume forward slashes while os.Getwd() returns backslashed paths:

  1. The buildPath helper shared by the background, compression, customfont and disablepagebreak examples and by the internal/cache and pkg/fontrepository tests trims the working directory with a forward-slash strings.ReplaceAll that never matches on Windows, so the asset path points inside the test directory and the font/image file is not found. The helper now walks the right number of segments up with filepath.Join, which reaches the repository root on every platform.
  2. pkg/core carries the same helper with a leftover substring from a package move (pkg/core/entity), which matches on no platform; the dead replacement is dropped.
  3. pkg/test locates .maroto.yml by splitting the working directory on /. On Windows the walk collapses to the filesystem root, the config is never loaded, and the suite panics on the nil singleton. Normalizing the working directory with filepath.ToSlash keeps the existing forward-slash walk working everywhere, since the Windows file APIs accept forward slashes.

Test-only change apart from the one line in pkg/test; no behaviour change on Linux (the CI path). Verified on Windows 11: go test ./... passes with this change, and fails in the three ways above without it.

Context: CodeRabbit flagged the non-portable buildPath copy that #604 adds; rather than fix one copy and leave five, #604 adopts the portable form for its own example and this PR aligns the existing ones.

Related Issue

Closes #606. Follow-up to the review discussion on #604.

Checklist

check with "x", ONLY IF APPLIED to your change

  • All methods associated with structs has func (<first letter of struct> *struct) method() {} name style.
  • Wrote unit tests for new/changed features.
  • Followed the unit test when,should naming pattern.
  • All mocks created with m := mocks.NewConstructor(t).
  • All mocks using m.EXPECT().MethodName() method to mock methods.
  • Updated docs/*
  • Updated example_test.go.
  • Updated README.md
  • New public methods/structs/interfaces has comments upside them explaining they responsibilities
  • Executed make dod with none issues pointed out by golangci-lint

Several test path constructions assumed forward slashes and broke under
the backslashed paths os.Getwd returns on Windows:

- the buildPath helper shared by the background, compression, customfont
  and disablepagebreak examples and by the internal/cache and
  pkg/fontrepository tests cut the working directory down with a
  forward-slash string replacement that never matched, so the asset path
  pointed inside the test directory. Walking the right number of
  segments up with filepath.Join reaches the repository root on every
  platform.
- pkg/core carried the same helper with a substring left over from a
  package move, dead on every platform; the replacement is dropped.
- pkg/test walked up to .maroto.yml by splitting on forward slashes, so
  on Windows the walk collapsed to the filesystem root, the config was
  never loaded and the suite panicked on the nil singleton. Normalizing
  the working directory with filepath.ToSlash keeps the existing
  forward-slash walk working everywhere, since the Windows file APIs
  accept forward slashes.

With these, go test ./... passes on Windows.
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0810ded9-fe69-4a6e-ad6b-db4a7b707768

📥 Commits

Reviewing files that changed from the base of the PR and between 242b124 and 47db31e.

📒 Files selected for processing (8)
  • docs/assets/examples/background/v2/main_test.go
  • docs/assets/examples/compression/v2/main_test.go
  • docs/assets/examples/customfont/v2/main_test.go
  • docs/assets/examples/disablepagebreak/v2/main_test.go
  • internal/cache/cache_test.go
  • pkg/core/pdf_test.go
  • pkg/fontrepository/fontrepository_test.go
  • pkg/test/test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved file path handling across tests and example assets for better cross-platform compatibility.
    • Updated fixture and configuration path resolution to work consistently across operating systems.

Walkthrough

Test helpers now construct fixture paths with filepath. Configuration lookup normalizes working-directory paths with filepath.ToSlash. These changes replace fixed forward-slash string handling across example, cache, PDF, font repository, and configuration tests.

Changes

Windows test path handling

Layer / File(s) Summary
Fixture path helper updates
docs/assets/examples/*/v2/main_test.go, internal/cache/cache_test.go, pkg/core/pdf_test.go, pkg/fontrepository/fontrepository_test.go
Test helpers use filepath.Join and directory traversal instead of fixed-prefix string replacement and path.Join.
Configuration path normalization
pkg/test/test.go
getMarotoConfigFilePath converts the working directory with filepath.ToSlash before appending the trailing separator.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 47db3

This PR makes test asset and configuration path handling Windows-safe without changing production behavior. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: johnfercher

Poem

A rabbit hops through paths anew
With slashes shaped for every view
Fixtures find their files in place
Config paths keep a steady pace
Windows tests now join the race

🚥 Pre-merge checks | ✅ 4 | ❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 8 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: making test-suite path handling safe on Windows.
Description check ✅ Passed The description explains the problem, technical changes, expected behavior, related issue, testing result, and applicable checklist item. It is complete and relevant.
Linked Issues check ✅ Passed The changes address all three causes in issue #606: non-portable buildPath helpers, the obsolete pkg/core replacement, and pkg/test configuration-path handling on Windows.
Out of Scope Changes check ✅ Passed The changes are limited to test path handling and the related pkg/test helper. They support the Windows test-suite objective in issue #606 and do not introduce unrelated scope.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@AmmrFX AmmrFX mentioned this pull request Aug 26, 2026
10 tasks
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.01%. Comparing base (242b124) to head (47db31e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #607   +/-   ##
=======================================
  Coverage   96.01%   96.01%           
=======================================
  Files          65       65           
  Lines        2479     2479           
=======================================
  Hits         2380     2380           
  Misses         64       64           
  Partials       35       35           
Flag Coverage Δ
unittests 96.01% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@johnfercher
johnfercher merged commit 8ed1d92 into johnfercher:master Sep 10, 2026
6 checks passed
@johnfercher

Copy link
Copy Markdown
Owner

Thanks!

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.

Test suite cannot run on Windows

2 participants