-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Make sure working directory is respected when configured from matrix #1686
Make sure working directory is respected when configured from matrix #1686
Conversation
@krisgeus this pull request has failed checks 🛠 |
@act-maintainers. The checks are failing but to mee it seem to be unrelated to my specific change. Unfortunately I wasn’t able to get the tests to run locally to verify. |
Seems like this change cause regressions, maybe you skipped over some function reading workdir
One error can be reproduced like, switching to act master fixes it
|
@ChristopherHX I see there is some kind of regression indeed. The test fails in my branch and not in master. I cannot figure out why and need help with that. I removed the unwanted side effect of setting the WorkingDirectory on the step (unwanted in the case of using a matrix) but it seems that this side effect is needed in regular cases. |
Codecov Report
@@ Coverage Diff @@
## master #1686 +/- ##
==========================================
+ Coverage 61.22% 62.83% +1.60%
==========================================
Files 46 48 +2
Lines 7141 7482 +341
==========================================
+ Hits 4372 4701 +329
+ Misses 2462 2461 -1
- Partials 307 320 +13
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you for contributing a fix.
Happy to be able to contribute. I updated the branch to match the latest master. Let me know if I need to do anything else to get this one merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. Thanks for your contribution 😃
fixes #1685