Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[cp] Add test for cycle symlink #1045

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wafuwafu13
Copy link
Contributor

Related: #739

@@ -621,26 +621,20 @@ test('Test max depth.', t => {
// Check last directory to exist is below maxdepth.
t.truthy(shell.test('-d', `${t.context.tmp}/copytestdepth${directory32deep}`));
t.falsy(shell.test('-d', `${t.context.tmp}/copytestdepth${directory32deep}/32`));
utils.skipOnWinForEPERM(shell.ln.bind(shell, '-s', `${t.context.tmp}/0`, `${t.context.tmp}/symlinktest`), () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delete it because it doesn't create symlinks to check for cycle.

@wafuwafu13
Copy link
Contributor Author

wafuwafu13 commented Sep 23, 2021

test passes in my local environment... 😓

@nfischer nfischer added the test label Jan 27, 2022
@nfischer
Copy link
Member

Could you rebase? I recently updated CI to use GitHub actions instead. This may be more reliable than the old environment.

@codecov-commenter
Copy link

codecov-commenter commented Feb 1, 2022

Codecov Report

Merging #1045 (b535d24) into master (9a0e5f6) will decrease coverage by 0.14%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1045      +/-   ##
==========================================
- Coverage   97.19%   97.04%   -0.15%     
==========================================
  Files          36       36              
  Lines        1354     1354              
==========================================
- Hits         1316     1314       -2     
- Misses         38       40       +2     
Impacted Files Coverage Δ
src/cp.js 98.46% <ø> (+6.92%) ⬆️
src/which.js 80.00% <0.00%> (-15.00%) ⬇️
src/cmd.js 94.11% <0.00%> (-5.89%) ⬇️
src/find.js 95.45% <0.00%> (-4.55%) ⬇️
src/ln.js 96.87% <0.00%> (-3.13%) ⬇️
src/ls.js 97.67% <0.00%> (-2.33%) ⬇️

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 9a0e5f6...b535d24. Read the comment docs.

@wafuwafu13
Copy link
Contributor Author

I don't know how to solve this problem in windows, so close

@wafuwafu13 wafuwafu13 closed this Feb 1, 2022
@nfischer
Copy link
Member

nfischer commented Feb 4, 2022

Thanks for trying the rebase. Looks like the error message from https://github.com/shelljs/shelljs/runs/5018531671?check_suite_focus=true is:

cp » Test with cycle symlink

  D:\a\shelljs\shelljs\tmp04812076144037267068[653](https://github.com/shelljs/shelljs/runs/5018531671?check_suite_focus=true#step:5:653)06818777819\test\cp.js:635

  Value is not falsy:

  'cp: cannot create directory \'new\': No such file or directory'

  _utils.default.skipOnWinForEPERM (test/cp.js:635:9)
  Object.skipOnWinForEPERM (test/utils/utils.js:31:5)
  t (test/cp.js:629:11)

I wonder if new is a reserved word on Windows, or maybe we just can't create the dir for some reason?

@wafuwafu13 wafuwafu13 reopened this Feb 6, 2022
@wafuwafu13
Copy link
Contributor Author

'cp: cannot create directory \'newsub\': No such file or directory' 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants