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

Drop broken symlinks #93

Merged
merged 13 commits into from
Oct 1, 2024
Merged

Drop broken symlinks #93

merged 13 commits into from
Oct 1, 2024

Conversation

RichardTribe
Copy link
Contributor

@RichardTribe RichardTribe commented Sep 17, 2024

🧐 What?
Aim: handle repos that contain broken symlinks

The 'recursive-readdir' library that we have been using still does not handle broken symlinks so nothing gets generated/uploaded.

If there are broken symlinks to files then skip them.

Still need to decide how to handle broken symlinks to dirs...or in fact good symlinks to dirs...
Done a manual test with a symlink to a dir and it seems to be just ignored.
I think this is reasonable :-)

Added replacement for 'recursive-readdir' that just uses node's fs

🛠 How

  • make use of the recursive option available from node 18+ in fs.readdirSync

  • unit test is added.

    • requires mocking with things like
      new Dirent('directory', constants.UV_DIRENT_DIR)
      which is do-able...
    • something wierd with the Jest mocking setup I feel it would fail with maeningless errors then work again
    • seems to be stable for any given version of the test file

🐞 Related issue

https://jira.dev.bbc.co.uk/browse/CLOUDPLAT-432

@RichardTribe RichardTribe marked this pull request as ready for review September 18, 2024 12:40
…e mocked and we can have more confidence in the tests.

Had to also add more logic around symlinks
@andysteadbbc andysteadbbc changed the title DRAFT: Drop broken symlinks Drop broken symlinks Oct 1, 2024
@andysteadbbc andysteadbbc merged commit 179d6f2 into main Oct 1, 2024
2 checks passed
@andysteadbbc andysteadbbc deleted the drop-broken-symlinks branch October 1, 2024 09:59
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.

2 participants