pharo-project / pharo-launcher

Lets you manage your pharo images and download new ones
https://pharo-project.github.io/pharo-launcher/
MIT License
108 stars 46 forks source link

Added exception handling for creating directory in package command #644

Closed Bajger closed 5 months ago

Bajger commented 5 months ago
Bajger commented 5 months ago

@demarey I don't know how to write unit test for that, since test class uses memory file system. Not sure if I can inject non-writeable directory (e.g. by using permissions?) and if memory store verifies that.

demarey commented 5 months ago

yes, you're right, it is hard to test this. We would need a test on the real file system for this purpose.

Bajger commented 5 months ago

@demarey CI is failing, do you have idea why? https://github.com/pharo-project/pharo-launcher/actions/runs/7553602942/job/20565517942#step:6:35

GitHub
Added exception handling for creating directory in package command · pharo-project/pharo-launcher@826b279
Lets you manage your pharo images and download new ones - Added exception handling for creating directory in package command · pharo-project/pharo-launcher@826b279
demarey commented 5 months ago

yes, because the test report is not accessible in the job when you are in a PR: https://github.com/pharo-project/pharo-launcher/actions/runs/7553602942/job/20565517942#step:6:35 If you take a look before, you can see all tests are passing :)

GitHub
Added exception handling for creating directory in package command · pharo-project/pharo-launcher@826b279
Lets you manage your pharo images and download new ones - Added exception handling for creating directory in package command · pharo-project/pharo-launcher@826b279
demarey commented 5 months ago

I will need to see how to solve this issue for PRs

Bajger commented 5 months ago

Should I merge it? Or wait until test report is accessible for PR?

demarey commented 5 months ago

you can merge it