Skip to content

Conversation

@Jasha10
Copy link
Contributor

@Jasha10 Jasha10 commented Nov 16, 2024

Description

This PR enables some tests that were disabled on macos.

We shall see if the CI passes. (Update: CI has passed.)

User-Facing Changes

Should be no user-facing changes as only a test-file is modified.

Tests + Formatting

Test coverage should increase

@Jasha10 Jasha10 marked this pull request as ready for review November 16, 2024 18:58
@Jasha10
Copy link
Contributor Author

Jasha10 commented Nov 16, 2024

I'm not sure why these tests were disabled on macos originally.

@dmatos2012, do you recall why that was?
(Pinging you because I see you wrote the tests)

Copy link
Contributor

@WindSoilder WindSoilder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the pr! Let's wait the response from @dmatos2012

@fdncred
Copy link
Contributor

fdncred commented Dec 1, 2024

if it breaks something we can revert. dmatos doesn't seem to be around.

@fdncred fdncred merged commit c4b919b into nushell:main Dec 1, 2024
13 checks passed
@github-actions github-actions bot added this to the v0.101.0 milestone Dec 1, 2024
@Jasha10 Jasha10 deleted the jasha10/enable-test_cp_recurse-on-macos branch December 3, 2024 01:12
@dmatos2012
Copy link
Contributor

Sorry for the late reply, even after merged, but this was so long ago I do not recall why it was disabled. Normally, I would have left a comment explaining why, so either a) It was disabled by accident which I doubt or most likely 2) There was a particular behavior back then that broke it and/or was also a known behavior in the uutils repo

In any case, if it doesnt fail now, then lets go for it :)

@fdncred fdncred added the tests issues to add tests or fix tests label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests issues to add tests or fix tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants