Skip to content

Disable tests on Windows that use ed#50

Merged
sylvestre merged 1 commit intouutils:mainfrom
cakebaker:disable_tests_using_ed_on_windows
Apr 4, 2024
Merged

Disable tests on Windows that use ed#50
sylvestre merged 1 commit intouutils:mainfrom
cakebaker:disable_tests_using_ed_on_windows

Conversation

@cakebaker
Copy link
Copy Markdown
Contributor

This PR disables three tests on Windows which currently fail because there is no ed program. It's related to #39, though doesn't fix it.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.82%. Comparing base (d863fe4) to head (2d9e625).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   76.79%   76.82%   +0.02%     
==========================================
  Files           9        9              
  Lines        2564     2567       +3     
  Branches      657      657              
==========================================
+ Hits         1969     1972       +3     
  Misses        466      466              
  Partials      129      129              
Flag Coverage Δ
macos_latest 77.00% <100.00%> (+0.02%) ⬆️
ubuntu_latest 76.86% <100.00%> (+0.02%) ⬆️

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

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

@sylvestre sylvestre merged commit 096aa1d into uutils:main Apr 4, 2024
@cakebaker cakebaker deleted the disable_tests_using_ed_on_windows branch April 4, 2024 06:49
@marc-hb
Copy link
Copy Markdown

marc-hb commented May 7, 2026

No big deal but I think it would be better not to give Windows users the wrong impression that these tests are passing on Windows. I mean like this instead:

--- a/src/ed_diff.rs
+++ b/src/ed_diff.rs
@@ -181,6 +181,7 @@ mod tests {
     }
 
     #[test]
+    #[cfg_attr(target_os = "windows", ignore = "no 'ed' on windows")]
     fn test_permutations() {
         let target = "target/ed-diff/";
         // test all possible six-line files.

This prints the following instead of making the tests silently disappear on Windows:

test ed_diff::tests::test_permutations ... ignored, no 'ed' on windows

I can submit a PR but reverting 2d9e625 would unfortunately git conflict with #226 - so I won't submit anything yet.

PS: following #226, it would be even better to dynamically skip based on whether ed is found or not. After all, ed can be missing on Linux too! See #39 (comment). But that would require "upgrading" to a non-std test crate:

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.

3 participants