Skip to content

Conversation

@lwid
Copy link
Contributor

@lwid lwid commented Nov 15, 2025

As pointed out in #166 there are some tests which still fail on Windows due to various reasons. This PR makes the test suite pass.

  • The os.sep commit should be trivial to understand.
  • For the CLI tests it was not, as expected, the newlines causing problems but rather it was the Table generator using different sets of unicode chars for rendering the table on win32 compared to other OS:s. This seems to be a known issue with rich. These test we modified to be more tolerant and less strict on formatting.
  • The redownload test fails due to the sqlite file being opened by the test harness which causes the unlink to fail. Getting this to work would require severe refactoring so I added a condiitional skip for this very test when running on win32. Out of the options I could think of this seems the most reasonable on.

With these three commits I now get a clean pytest run on Windows (with explicit skipping of the redownload test in the log).

Please verify that the tests function as before on your prefered platforms and that you are fine with the modified CLI tests.

Linus W. added 3 commits November 15, 2025 11:49
…unicode/ascii characters on different platforms which cause the existing tests to fail when running tests on Windows. Instead of comparing the outbyte byte-wise the modified tests checks that all expected data is present in the expectes lines which, in combination with the JSON testing, should be strict enough to detect breaking changes in the CLI output
…issionError: [WinError 32] on the unlink due to pytest having the sqlite file open already. This is specific for the test setup and remediating would require quite some refactoring. Better to skip the test on win32 and catching any problems in the build pipeline. By marking it as a conditional skip we can achieve 100% test success on win32, with the s markers pointing out that these two tests were activly skipped
@codecov
Copy link

codecov bot commented Nov 15, 2025

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 97.91%. Comparing base (a1e3de0) to head (79b84a7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
actual/__init__.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #167   +/-   ##
=======================================
  Coverage   97.90%   97.91%           
=======================================
  Files          19       19           
  Lines        2677     2680    +3     
=======================================
+ Hits         2621     2624    +3     
  Misses         56       56           

☔ View full report in Codecov by Sentry.
📢 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.

@bvanelli
Copy link
Owner

After some headache with the sqlite3 library, the last test is now fixed for Windows (at my machine). Turns out, if you don't call the conn.close() after leaving the context manager, it doesn't do anything on Windows.

The codecov we can ignore, it likes to complain about single uncovered lines.

Could you confirm it also works in your machine too? Then we can merge it.

@lwid
Copy link
Contributor Author

lwid commented Nov 18, 2025

Verified, works perfectly and now I get a 100% Pass ratio. Nice!

@bvanelli bvanelli merged commit 10fe107 into bvanelli:main Nov 18, 2025
8 of 9 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.

2 participants