Add Test Results Summary for compatibility tests #53

mpanchajanya · 2023-04-12T14:03:41Z

What this PR does / why we need it

Filter the compatibility test logs
Add Test Results summary for github workflow for Compatibility tests.
Update ginkgo to ginkgo/v2

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Filtered logs in Github workflow
New Test Results Summary section in the bottom section of the workflow
When a user clicks on the badge displayed in the root README, they will be directed to the workflow runs on the main branch. By selecting the most recent run, they can view the Test Results Summary.

Tested make run-compatibility-tests and exit code with '0' representing success and non-zero values indicating test failures

Ginkgo ran 3 suites in 40.44465325s
Test Suite Passed
mpanchajanya@mpanchajanF09MK tanzu-plugin-runtime % echo $?                     
0

Ginkgo ran 3 suites in 49.492652209s

There were failures detected in the following suites:
  context ./test/compatibility/framework/compatibilitytests/context

Test Suite Failed
make: *** [run-compatibility-tests] Error 1
mpanchajanya@mpanchajanF09MK tanzu-plugin-runtime % echo $?
2

mpanchajanya@mpanchajanF09MK tanzu-plugin-runtime % make help

Usage:
  make <target>
  help                          Display this help (default)
  all                           Run all major targets (lint, test, modules)
  test                          Run Tests
  fmt                           Run goimports
  lint                          Run linting and misspell checks
  go-lint                       Run linting of go source
  doc-lint                      Run linting checks for docs
  modules                       Runs go mod to ensure modules are up to date.
  tools                         Build tooling binaries
  clean                         Remove all generated binaries
  compatibility-tests           Build and Run Compatibility tests
  build-compatibility-test-plugins  Builds all runtime compatibility test plugins
  run-compatibility-tests       Run Compatibility tests

Release note

Additional information

Special notes for your reviewer

anujc25

This looks great to me. Thanks for improving the test result summary.
I wait for @vuil to take a look at this PR before we merge this pr to get his opinion as well.

marckhouzam

Cool! Very nice look.
A couple of comments:

if you run make modules one of the go.sum file is not up-to-date
Would it be possible to work the output to avoid putting the big red X when there are 0 failures?

marckhouzam · 2023-04-17T14:10:02Z

.github/workflows/compatibility_tests.yaml


      - name: Build Runtime Test Plugins
        run: make build-compatibility-test-plugins

      - name: Run Compatibility Tests
-        run: make run-compatibility-tests
+        run: cd ./test/compatibility/framework/compatibilitytests && ginkgo --keep-going --race -r -v --randomize-all --trace --output-dir ./../../../../testresults --json-report=compatibility-tests.json > /tmp/out && { cat /tmp/out | grep -Ev 'STEP:|seconds|.go:'; rm /tmp/out; } || { exit_code=$?; cat /tmp/out | grep -Ev 'STEP:|seconds|.go:'; rm /tmp/out; exit $exit_code; }
+


There is a bit of duplication here and with the make run-compatibility-tests target. Was replacing the make target here just for testing, or is it the final iteration because it needs to do different things than the make target?

Addressed the duplication using the same target

marckhouzam · 2023-04-17T14:13:31Z

.github/workflows/compatibility_tests.yaml

+      - name: Tests Results Summary
+        if: always()
+        run: |
+          chmod +x ./hack/scripts/process-ginkgo-test-results.sh


Do we need this? Isn't the script checked-in as executable?

No longer needed. Updated the script

marckhouzam · 2023-04-17T14:17:06Z

hack/scripts/process-ginkgo-test-results.sh

+    echo "| $suite_description | $suite_tests | :white_check_mark: $suite_passed | :x: $suite_failed |"
+  else
+    echo "| $suite_description | $suite_tests | :white_check_mark: $suite_passed | :x: $suite_failed |"
+  fi


I see you were planning on having a different behaviour when there was no suite_failed, but the echo is the same for both cases. Was that intentional?

Initial thought was to show slight different on failed. maybe be we could exclude the X icon if no failures. Updated accordingly

marckhouzam · 2023-04-17T14:17:20Z

hack/scripts/process-ginkgo-test-results.sh

+  echo "| **Total** | $total_tests | :white_check_mark: $total_passed | :x: $total_failed |"
+else
+  echo "| **Total** | $total_tests | :white_check_mark: $total_passed | :x: $total_failed |"
+fi


Here too the echo's are both the same, was that intentional?

Initial thought was to show slight different on failed. maybe be we could exclude the X icon if no failures. Updated accordingly

chandrareddyp

Great work!! LGTM!
Couple of comments added.

chandrareddyp · 2023-04-18T15:39:25Z

hack/scripts/process-ginkgo-test-results.sh

+
+  # Print the suite row with color and icon depending on the result
+  if [ "$suite_failed" -eq 0 ]; then
+    echo "| $suite_description | $suite_tests | :white_check_mark: $suite_passed | $suite_failed |"


Looks you are not add X (red color) if no failures in the suite, but in the output attached in the PR description has X (red color) even there are no failures.

Updated the images to reflect the results with changes

chandrareddyp · 2023-04-18T15:40:38Z

hack/scripts/process-ginkgo-test-results.sh

+json=$(cat "$1")
+
+# Print the table header
+echo "| :memo: Test Suite Description | Total Tests | :white_check_mark: Passed | :x: Failed |"


Suggestion, how about not adding X (red color) on the header next to Failed, add X (red color) to the results which has some failures!

So were the changes updated? If so, might be good to update the screenshots in the description with the latest output.

Nit: my similar comment is I don't find the emoticons add much to aid comprehending the results; I would even say they are rather distracting tbh. It might be sufficient to highlight when some tests fail, and leave the rest of the tabular data rather plain.

Makes sense. Updated the images to reflect the results with changes. I have remove the green check and kept red X only if there are any failures

Makefile

…he test script logs

vuil · 2023-04-19T21:07:35Z

Makefile

@@ -136,9 +139,7 @@ build-compatibility-test-plugins: ## Builds all runtime compatibility test plugi
 	cd ./test/compatibility/testplugins/runtime-test-plugin-v0_28_0 && ${GO} mod tidy && GOOS=$(OS) GOARCH=$(ARCH) ${GO} build -o ../bin
 	cd ./test/compatibility/testplugins/runtime-test-plugin-latest && ${GO} mod tidy && GOOS=$(OS) GOARCH=$(ARCH) ${GO} build -o ../bin

+## The below command runs the compatibility tests using ginkgo and filter the logs as per regex and exit code with '0' representing success and non-zero values indicating test failures


single # is sufficient. ## is convention for the make help, not relevant here.

@vuil Made the changes

vuil

lgtm

* Add GitHub Summary to present compatibility test results and filter the test script logs * Bump ginkgo and gomega versions * Remove icons to table data * Update the compatibility test command and description

vmwclabot added the cla-not-required label Apr 12, 2023

mpanchajanya changed the title ~~WIP Present test results~~ (WIP) (DRAFT) Present test results Apr 12, 2023

mpanchajanya force-pushed the present_test_results branch 2 times, most recently from f6853db to d677beb Compare April 13, 2023 18:17

mpanchajanya mentioned this pull request Apr 13, 2023

Cross-version API compatibility tests - Filter the test results logs #41

Closed

mpanchajanya force-pushed the present_test_results branch 2 times, most recently from d692e1a to 14cb17c Compare April 13, 2023 20:36

mpanchajanya changed the title ~~(WIP) (DRAFT) Present test results~~ (DRAFT) Present test results Apr 13, 2023

mpanchajanya changed the title ~~(DRAFT) Present test results~~ Add Test Results Summary for compatibility tests Apr 14, 2023

mpanchajanya marked this pull request as ready for review April 14, 2023 16:08

mpanchajanya requested a review from a team as a code owner April 14, 2023 16:08

anujc25 reviewed Apr 14, 2023

View reviewed changes

marckhouzam reviewed Apr 17, 2023

View reviewed changes

mpanchajanya force-pushed the present_test_results branch 2 times, most recently from 54d5a11 to d9e9149 Compare April 17, 2023 18:47

chandrareddyp approved these changes Apr 18, 2023

View reviewed changes

mpanchajanya force-pushed the present_test_results branch 3 times, most recently from 16c0098 to 26a5af9 Compare April 19, 2023 04:21

mpanchajanya requested a review from vuil April 19, 2023 14:13

vuil reviewed Apr 19, 2023

View reviewed changes

Makefile Outdated Show resolved Hide resolved

mpanchajanya added 3 commits April 19, 2023 13:02

Add GitHub Summary to present compatibility test results and filter t…

88193dd

…he test script logs

Bump ginkgo and gomega versions

0be8b96

Remove icons to table data

2a0b9e8

mpanchajanya force-pushed the present_test_results branch 3 times, most recently from bad9c9a to b010eb6 Compare April 19, 2023 20:32

vuil reviewed Apr 19, 2023

View reviewed changes

Update the compatibility test command and description

c430950

mpanchajanya force-pushed the present_test_results branch from b010eb6 to c430950 Compare April 20, 2023 02:39

mpanchajanya requested a review from vuil April 20, 2023 02:40

vuil approved these changes Apr 20, 2023

View reviewed changes

mpanchajanya merged commit 68c4941 into vmware-tanzu:main Apr 20, 2023
4 checks passed

marckhouzam added this to the v0.90.0 milestone Oct 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Test Results Summary for compatibility tests #53

Add Test Results Summary for compatibility tests #53

mpanchajanya commented Apr 12, 2023 •

edited

anujc25 left a comment

marckhouzam left a comment

marckhouzam Apr 17, 2023

mpanchajanya Apr 19, 2023

marckhouzam Apr 17, 2023

mpanchajanya Apr 19, 2023

marckhouzam Apr 17, 2023

mpanchajanya Apr 18, 2023

marckhouzam Apr 17, 2023

mpanchajanya Apr 17, 2023 •

edited

chandrareddyp left a comment

chandrareddyp Apr 18, 2023

mpanchajanya Apr 19, 2023 •

edited

chandrareddyp Apr 18, 2023

vuil Apr 18, 2023 •

edited

mpanchajanya Apr 19, 2023

vuil Apr 19, 2023

mpanchajanya Apr 20, 2023 •

edited

vuil left a comment

Add Test Results Summary for compatibility tests #53

Add Test Results Summary for compatibility tests #53

Conversation

mpanchajanya commented Apr 12, 2023 • edited

What this PR does / why we need it

Which issue(s) this PR fixes

Describe testing done for PR

Release note

Additional information

Special notes for your reviewer

anujc25 left a comment

Choose a reason for hiding this comment

marckhouzam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpanchajanya Apr 17, 2023 • edited

Choose a reason for hiding this comment

chandrareddyp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpanchajanya Apr 19, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vuil Apr 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpanchajanya Apr 20, 2023 • edited

Choose a reason for hiding this comment

vuil left a comment

Choose a reason for hiding this comment

mpanchajanya commented Apr 12, 2023 •

edited

mpanchajanya Apr 17, 2023 •

edited

mpanchajanya Apr 19, 2023 •

edited

vuil Apr 18, 2023 •

edited

mpanchajanya Apr 20, 2023 •

edited