UID2-3858 verify no of tests run #141
Conversation
| run: | | ||
| cd ./${{ inputs.working_dir }} | ||
| mvn -B -s settings.xml -Dgpg.passphrase="${{ secrets.GPG_PASSPHRASE }}" clean compile test | ||
| mvn -B -s settings.xml -Dgpg.passphrase="${{ secrets.GPG_PASSPHRASE }}" clean compile test | tee build.log |
There was a problem hiding this comment.
these sections are almost identical. Can we move to scripts folder with the extra flags
"-s settings.xml -Dgpg.passphrase="${{ secrets.GPG_PASSPHRASE }}"
as an argument
cYKatherine
force-pushed
the
kcc-UID2-3858-verify-no-of-tests-run
branch
from
e0b111a to
9165378
Compare
| - name: Checkout uid2-shared-actions repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: kcc-UID2-3858-verify-no-of-tests-run |
There was a problem hiding this comment.
Change to v3 before merging
There was a problem hiding this comment.
Is the checkout of the repo necessary? Is it possible to use a reusable workflow instead?
There was a problem hiding this comment.
It was required for testing. Keep it here as Abu is doing some refactoring laster on so that we don't need to specify the branch name every where when we are testing in the future.
| - name: Checkout uid2-shared-actions repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: kcc-UID2-3858-verify-no-of-tests-run |
There was a problem hiding this comment.
Change to v3 before merging
| echo "WARNING: Could not determine the number of tests run." | ||
| tests_run=0 |
There was a problem hiding this comment.
Should indent here.
Should we just exit here? If not, the output will be confusing:
WARNING: Could not determine the number of tests run.
ERROR: No tests were run!
| echo "ERROR: No tests were run!" | ||
| exit 1 |
| - name: Checkout uid2-shared-actions repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: kcc-UID2-3858-verify-no-of-tests-run |
There was a problem hiding this comment.
Is the checkout of the repo necessary? Is it possible to use a reusable workflow instead?
| @@ -0,0 +1,19 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| mvn -B $EXTRA_FLAGS clean compile test | tee build.log | |||
There was a problem hiding this comment.
Is there a reason you didn't use parameters to the script, rather than environment variables? It would seem to me to be more common to use parameters - however I am not strongly opinionated here
thomasm-ttd
left a comment
thomasm-ttd
left a comment
There was a problem hiding this comment.
Small comment re environment variables vs parameters, but happy for you to make a call
No description provided.