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

Add workingDirectory input #220

Merged
merged 2 commits into from
Sep 22, 2020
Merged

Add workingDirectory input #220

merged 2 commits into from
Sep 22, 2020

Conversation

arareko
Copy link
Contributor

@arareko arareko commented Sep 2, 2020

When checking out multiple repos within a single GitHub workflow, the use of path becomes necessary to keep things organized, but also creates the need to move across directories to accomplish certain tasks. In the case of running unit tests for a specific project folder, this usually leaves the resulting files in the directories where the test command was executed.

Code Climate's cc-test-report binary does not support pre-pending the path to the "location" of the coverage files, and also searches for a .git/ directory in the current working directory. These two conditions make it necessary to run the binary within the proper location.

GitHub actions’ uses step does not allow specifying a working-directory in which to run the action being "used". At the same time, the uses step doesn't seem to execute a shell (thus the run item does not apply to it), hence the cd shell builtin is never available. Pre-pending cd <path> && to the coverageCommand input results in an error due to this.

This patch adds a workingDirectory input to this GH action, which allows changing to a custom working directory via Node.js’ process.chdir() function. It also fixes a few places where error() was being passed non-string values, thus giving runtime errors.

@arareko arareko marked this pull request as draft September 2, 2020 06:10
@arareko arareko marked this pull request as ready for review September 2, 2020 06:21
@arareko
Copy link
Contributor Author

arareko commented Sep 9, 2020

Hi @paambaati, any chance you can have a look at this? Thanks!

@arareko
Copy link
Contributor Author

arareko commented Sep 16, 2020

Hi @vladjerca, @mattvv & @MartinNuc, can any of you take a look at this? Thanks!

Copy link
Contributor

@vladjerca vladjerca left a 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 👍

@vladjerca
Copy link
Contributor

@arareko Nice addition, but it's on @paambaati to get this merged, don't know who else has merge rights on the repo 😋

@arareko
Copy link
Contributor Author

arareko commented Sep 16, 2020

@vladjerca thanks for the comment! hopefully @paambaati gets to see this then..

Copy link
Contributor

@MartinNuc MartinNuc left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@paambaati
Copy link
Owner

I'm so sorry folks, I'd been busy with work. I'm looking at this PR right now and you can expect it to be merged in the next few hours.

@paambaati paambaati self-assigned this Sep 22, 2020
@paambaati paambaati added the enhancement New feature or request label Sep 22, 2020
src/main.ts Show resolved Hide resolved
src/main.ts Outdated Show resolved Hide resolved
@paambaati paambaati merged commit 73ef252 into paambaati:master Sep 22, 2020
@paambaati
Copy link
Owner

@arareko Thanks a lot for this PR! I'll be following this up with tests.

@arareko arareko deleted the add-working-directory branch September 22, 2020 18:31
@arareko
Copy link
Contributor Author

arareko commented Sep 22, 2020

@arareko Thanks a lot for this PR! I'll be following this up with tests.

@paambaati I just saw you added the tests and released this feature along with a new version. Thanks so much. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants