enabled one test on Windows

[yegor256]

related to #537

Summary by CodeRabbit

• Tests

  • Refactored tests to use dynamically created temporary directories (portable across platforms)
  • Implemented automatic cleanup of created test directories after execution
  • Updated test file operations with improved path construction and directory handling

• Chores

  • Added a runtime test dependency to support timestamp-based temporary directory generation

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Warning

Rate limit exceeded

@yegor256 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 8 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between bcc07a2 and 63a8574.

📒 Files selected for processing (1)

• test/CLISpec.hs

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Added time ^>=1.12 to the test-suite build-depends and refactored CLI tests to use a POSIX-timestamped temporary directory for file creation and cleanup; removed Windows-specific skip logic.

Changes

Cohort / File(s)	Summary
Dependency Addition phino.cabal	Added time ^>=1.12 to the test-suite build-depends.
Test Infrastructure Refactor test/CLISpec.hs	Replaced OS-specific file-write branch with portable temp-dir approach: added imports for Data.Time.Clock.POSIX.getPOSIXTime, System.Directory.createDirectoryIfMissing, System.Directory.getTemporaryDirectory, and System.FilePath.(</>); create unique temp directory using POSIX timestamp, ensure directory exists, write target files inside it, and perform recursive cleanup; removed Windows-only pending skip.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A rabbit hops through tempfile glades so spry,
POSIX ticks, a folder born under sky.
Files are written, tests dance and then depart,
The broom sweeps up without a single mart. 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'enabled one test on Windows' accurately reflects the main change: making a previously Windows-incompatible test portable so it can run on Windows by replacing OS-dependent logic with a cross-platform approach.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/CLISpec.hs (1)

841-855: Excellent refactor for cross-platform test compatibility!

The test now uses a temporary directory strategy that works reliably across platforms, including Windows. The bracket pattern ensures proper cleanup even if the test fails, and using the system temp directory with </> for path construction is the correct cross-platform approach.

Optional: Consider adding randomness for highly parallel test environments

The current approach uses floor stamp :: Integer, which truncates to seconds. If multiple tests execute in parallel within the same second, directory name collisions could theoretically occur. Consider adding a small random component or using higher precision:

-            let dir = tmp </> ("phino-test-" ++ show (floor stamp :: Integer))
+            let dir = tmp </> ("phino-test-" ++ show (floor (stamp * 1000) :: Integer))

This multiplies by 1000 to include milliseconds in the directory name, making collisions extremely unlikely even in parallel execution.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ac17cc and 68d3f6f.

📒 Files selected for processing (2)

• phino.cabal
• test/CLISpec.hs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: Agent
• GitHub Check: stack (ubuntu-24.04)
• GitHub Check: stack (macos-15)
• GitHub Check: stack (windows-2022)
• GitHub Check: cabal (macos-15)
• GitHub Check: cabal (ubuntu-24.04)
• GitHub Check: cabal (windows-2022)

🔇 Additional comments (2)

phino.cabal (1)

166-166: LGTM! Dependency addition enables cross-platform test improvements.

The time dependency is correctly added to the test suite and matches the version constraint already used in the library dependencies (line 91), enabling the POSIX timestamp functionality used in the refactored test.

test/CLISpec.hs (1)

14-14: LGTM! Imports support cross-platform temporary directory handling.

The added imports enable the test refactor to use system temporary directories with proper path construction and cleanup, improving cross-platform compatibility.

Also applies to: 18-18, 20-20

[Copilot]

Pull request overview

This PR enables a previously Windows-skipped test by replacing the problematic openTempFile approach with a more robust temporary directory-based solution that avoids file locking issues on Windows.

• Replaces openTempFile with getTemporaryDirectory and custom directory creation
• Adds the time package dependency to generate unique directory names using POSIX timestamps
• Removes the Windows-specific conditional skip for the "writes to target file" test

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
test/CLISpec.hs	Refactored the "writes to target file" test to use a temporary directory instead of a temporary file in the current directory, eliminating Windows file locking issues
phino.cabal	Added time ^>=1.12 dependency to support POSIX timestamp generation for unique directory naming

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Using POSIX timestamp floored to an integer for directory naming could lead to race conditions if tests run in parallel within the same second. Consider using a more robust approach like System.IO.Temp's createTempDirectory which handles uniqueness atomically, or add a random component to the directory name to reduce collision probability.

[Copilot]

This test contains multiple assertions which violates the coding guideline. The test should be split into separate test cases, each with a single assertion. Consider creating two separate tests: one for verifying the documentclass and another for verifying the begin document marker.

[Copilot]

Blank line inside method body reduces code readability according to the coding guidelines. Remove this blank line.

[yegor256]

@maxonfjvipon the test was disabled on Windows. Now it's enabled and should work on all platforms, thus increasing the coverage.

[maxonfjvipon]

@rultor merge

[rultor]

@rultor merge

@maxonfjvipon OK, I'll try to merge now. You can check the progress of the merge here.

[rultor]

@rultor merge

@maxonfjvipon Done! FYI, the full log is here (took me 10min).