Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

CI: Add windows-latest to job matrix #519

Merged
merged 4 commits into from
Apr 29, 2022
Merged

CI: Add windows-latest to job matrix #519

merged 4 commits into from
Apr 29, 2022

Conversation

avdv
Copy link
Collaborator

@avdv avdv commented Apr 25, 2022

Description

Run tests on Windows, too.

  • Relevant Issues : (none)
  • Relevant PRs : (none)
  • Type of change :
    • New feature
    • Bug fix for existing feature
    • Code quality improvement
    • Addition or Improvement of tests
    • Addition or Improvement of documentation

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2022

Codecov Report

Merging #519 (dae7a54) into main (0c6928d) will increase coverage by 0.17%.
The diff coverage is 0.00%.

❗ Current head dae7a54 differs from pull request most recent head 8c73bd0. Consider uploading reports for the commit 8c73bd0 to get more accurate results

@@            Coverage Diff             @@
##             main     #519      +/-   ##
==========================================
+ Coverage   89.60%   89.77%   +0.17%     
==========================================
  Files           8        8              
  Lines         577      577              
==========================================
+ Hits          517      518       +1     
+ Misses         60       59       -1     
Impacted Files Coverage Δ
lib/colorls/git.rb 68.57% <0.00%> (ø)
lib/colorls/core.rb 90.72% <0.00%> (+0.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c6928d...8c73bd0. Read the comment docs.

@avdv avdv force-pushed the ci-on-windows branch 2 times, most recently from c66711b to 1431839 Compare April 26, 2022 06:44
When running on Windows this test failed with:

```
Failures:

  1) ColorLS::Flags symlinked directory with trailing separator shows the file in the linked directory
     Failure/Error: expect { subject }.to output(/yaml_sort_checker.rb/).to_stdout

       expected block to output /yaml_sort_checker.rb/ to stdout, but output "       Supportlink \n"
       Diff:
       @@ -1 +1 @@
       -/yaml_sort_checker.rb/
       +       Supportlink
     # ./spec/color_ls/flags_spec.rb:329:in `block (3 levels) in <top (required)>'
```

This is due to the different handling of paths to a symlink with a trailing
slash / backslash.

`File.lstat(x).directory?` returns

OS      |     x    | value
----------------------------
Windows | symlink\ | false
Windows | symlink  | false
Linux   | symlink/ | true
Linux   | symlink  | false

This could be fixed in colorls by handling a trailing (back)slash specifically,
but I am unsure whether that is the right thing to do on Windows.
@avdv avdv marked this pull request as ready for review April 29, 2022 14:11
@avdv avdv added the Windows specific to Windows label Apr 29, 2022
@avdv avdv merged commit c7aa167 into main Apr 29, 2022
@avdv avdv deleted the ci-on-windows branch April 29, 2022 14:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Windows specific to Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants