Skip to content

UID2-3858 verify no of tests run #141

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.

Sign up for GitHub

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

Merged
cYKatherine merged 7 commits into from
Dec 24, 2024
Merged

UID2-3858 verify no of tests run #141

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a1ca4d2
Add check for the number of tests run
cYKatherine Dec 19, 2024
dc4cc72
Add tests for number of tests run
cYKatherine Dec 19, 2024
6bac3f5
Extract scripts
cYKatherine Dec 23, 2024
9165378
Add checkout uid2-shared-actions step
cYKatherine Dec 23, 2024
709d546
Exit when tests_run doesn't have value
cYKatherine Dec 23, 2024
837cba8
Use parameter to pass in values
cYKatherine Dec 24, 2024
3a44684
Change kcc-UID2-3858-verify-no-of-tests-run to v3
cYKatherine Dec 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .github/workflows/shared-build-and-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ jobs:
steps:
- name: Checkout repo
uses: actions/checkout@v4

- name: Checkout uid2-shared-actions repo
uses: actions/checkout@v4
with:
ref: kcc-UID2-3858-verify-no-of-tests-run
Copy link
Contributor Author

@cYKatherine cYKatherine Dec 23, 2024

Choose a reason for hiding this comment

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

Change to v3 before merging

Copy link
Contributor

@mcollins-ttd mcollins-ttd Dec 23, 2024

Choose a reason for hiding this comment

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

Is the checkout of the repo necessary? Is it possible to use a reusable workflow instead?

Copy link
Contributor Author

@cYKatherine cYKatherine Dec 23, 2024

Choose a reason for hiding this comment

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

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.

repository: IABTechLab/uid2-shared-actions
path: uid2-shared-actions

- name: Set up JDK
if: ${{ inputs.vulnerability_scan_only == 'false' }}
Expand All @@ -38,8 +45,9 @@ jobs:

- name: Build and run unit tests
if: ${{ inputs.vulnerability_scan_only == 'false' }}
run: mvn -B clean compile test
working-directory: ${{ inputs.working_dir }}
run: |
bash uid2-shared-actions/scripts/compile_java_test_and_verify.sh

- name: Generate code coverage
if: ${{ inputs.vulnerability_scan_only == 'false' }}
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/shared-publish-to-maven-versioned.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ jobs:
with:
fetch-depth: 0

- name: Checkout uid2-shared-actions repo
uses: actions/checkout@v4
with:
ref: kcc-UID2-3858-verify-no-of-tests-run
Copy link
Contributor Author

@cYKatherine cYKatherine Dec 23, 2024

Choose a reason for hiding this comment

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

Change to v3 before merging

repository: IABTechLab/uid2-shared-actions
path: uid2-shared-actions

- name: Set up JDK
uses: actions/setup-java@v4
with:
Expand Down Expand Up @@ -113,9 +120,11 @@ jobs:

- name: Compile
if: ${{ inputs.publish_to_maven != true }}
env:
EXTRA_FLAGS: "-s settings.xml -Dgpg.passphrase=\"${{ secrets.GPG_PASSPHRASE }}\""
run: |
cd ./${{ inputs.working_dir }}
mvn -B -s settings.xml -Dgpg.passphrase="${{ secrets.GPG_PASSPHRASE }}" clean compile test
bash uid2-shared-actions/scripts/compile_java_test_and_verify.sh

- name: Commit pom.xml and version.json
if: ${{ steps.checkRelease.outputs.is_release != 'true' }}
Expand Down
19 changes: 19 additions & 0 deletions scripts/compile_java_test_and_verify.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env bash

mvn -B $EXTRA_FLAGS clean compile test | tee build.log
Copy link
Contributor

@thomasm-ttd thomasm-ttd Dec 23, 2024

Choose a reason for hiding this comment

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

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


tests_run=$(cat build.log | grep "Tests run:" | tail -n 1 | sed 's/.*Tests run: \([0-9]*\).*/\1/')

echo "DEBUG: tests_run = $tests_run"

if [ -z "$tests_run" ]; then
echo "WARNING: Could not determine the number of tests run."
tests_run=0
Copy link
Contributor

@mcollins-ttd mcollins-ttd Dec 23, 2024

Choose a reason for hiding this comment

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

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!

fi

if [ "$tests_run" -eq 0 ]; then
echo "ERROR: No tests were run!"
exit 1
Copy link
Contributor

@mcollins-ttd mcollins-ttd Dec 23, 2024

Choose a reason for hiding this comment

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

Indentation

fi

echo "INFO: $tests_run tests were run!"
Loading