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

test uninstallers #569

Merged
merged 7 commits into from
Sep 14, 2022
Merged

test uninstallers #569

merged 7 commits into from
Sep 14, 2022

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Sep 12, 2022

Description

We were not testing the uninstallers generated on Windows, so at least I want to make sure we can run them and the files are (mostly) deleted. Other side effects are not tested (like registry cleanup or menu item removal). Maybe in another PR, after maybe revamping the testing infrastructure so we use pytest or similar.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Sep 12, 2022
@jaimergp
Copy link
Contributor Author

We are running into some IO lock issues where ExecWait leaves a Python process open but returns control the uninstaller, so the temporary directory cleanup fails. We have seen reports of this problem in some issues, but hopefully #558 alleviates this problem a bit (in that PR we switched from ExecWait to nsExec which is a bit more robust).

@jaimergp
Copy link
Contributor Author

Actually it wasn't ExecWait but the self-copy mechanism of the uninstaller, which returns control immediately. Using _?=<path> prevents the copy and hence it doesn't return immediately.

Ready for review!

@jaimergp jaimergp marked this pull request as ready for review September 13, 2022 07:58
@jaimergp jaimergp requested a review from a team as a code owner September 13, 2022 07:58
Copy link

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Thanks @jaimergp

@larsoner larsoner merged commit 0722efc into conda:main Sep 14, 2022
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Sep 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants