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.

Sign up for GitHub

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

Jump to bottom

testsuite: use removeDirectoryRecursiveHack to cleanup #9055

Merged
merged 2 commits into from
Jun 22, 2023
Merged

testsuite: use removeDirectoryRecursiveHack to cleanup #9055

merged 2 commits into from
Jun 22, 2023

Conversation

ulysses4ever
Copy link
Collaborator

@ulysses4ever ulysses4ever commented Jun 21, 2023 •
edited
Loading

This is supposedly more robust on Windows than removeDirectoryRecursive.

Please include the following checklist in your PR:

Bonus points for added automated tests!

geekosaur
geekosaur approved these changes Jun 21, 2023
View reviewed changes
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

LGTM

@ulysses4ever ulysses4ever mentioned this pull request Jun 21, 2023
Merged
4 tasks
@ulysses4ever ulysses4ever added the merge me Tell Mergify Bot to merge label Jun 21, 2023
@ulysses4ever ulysses4ever force-pushed the removeDirectoryRecursiveHack-more branch from b720d16 to bd29c29 Compare June 21, 2023 18:59
@ulysses4ever
Copy link
Collaborator Author

@gbaz as discussed on IRC, I moved TempTestDir to Cabal.

@ulysses4ever ulysses4ever force-pushed the removeDirectoryRecursiveHack-more branch from 6fa6d64 to bd29c29 Compare June 21, 2023 19:52
@gbaz
Copy link
Collaborator

gbaz commented Jun 21, 2023

nice! care to take care of this along the way? #7549

@ulysses4ever
Copy link
Collaborator Author

You bet.

@ulysses4ever
Copy link
Collaborator Author

All right. There are about 3 PRs that are currently hit by failures on Windows related to temporary directories. I'm going to fast track this PR.

@ulysses4ever ulysses4ever added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jun 22, 2023
@ulysses4ever
move TempTestDir from cabal-install:unit-tests to Cabal
d5a272e 
That way we can reuse removeDirectoryRecursiveHack in cabal-testsuite.
@ulysses4ever
testsuite: use safer way to create temporary directory (fix #7549)
4085e3c 
Presumably, should be be more reliable on Windows.
@ulysses4ever ulysses4ever force-pushed the removeDirectoryRecursiveHack-more branch from 74ed534 to 4085e3c Compare June 22, 2023 11:18
@mergify mergify bot merged commit 20cab46 into master Jun 22, 2023
@mergify mergify bot deleted the removeDirectoryRecursiveHack-more branch June 22, 2023 13:38
@ulysses4ever ulysses4ever mentioned this pull request Jul 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gbaz gbaz gbaz left review comments

@ffaf1 ffaf1 ffaf1 approved these changes

@geekosaur geekosaur geekosaur approved these changes

Assignees
No one assigned
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ulysses4ever @gbaz @geekosaur @ffaf1