fix: Use PR head commit in artifact naming instead of merge commit #1643

Bowenislandsong · 2020-07-16T12:57:17Z

The env var GITHUB_SHA shows merge commit and is not related to worflow trigger unlike the discription on GITHUB. This fix changes the variable to look at the head commit aka. the commit that actually triggers the workflow.

Description of the change:

Motivation for the change:

Reviewer Checklist

Implementation matches the proposed design, or proposal is updated to match implementation
Sufficient unit test coverage
Sufficient end-to-end test coverage
Docs updated or added to /docs
Commit messages sensible and descriptive

Bowenislandsong · 2020-07-16T13:00:06Z

/hold
need to verify the uploaded artifact indeed contains a SHA value that trigged the workflow.

Bowenislandsong · 2020-07-16T13:37:58Z

/hold cancel
Artifact e2e-test-output-bbeb3644b061b5692ba87115113b0dcb1c7f12c8-171359250 has been successfully uploaded!

Bowenislandsong · 2020-07-16T13:41:49Z

The PR failed tests for 1 times with 1 individual failed tests and 5 skipped tests.

totaltestcount: 1
flaketestcount: 1
skippedtestcount: 5
flaketests:

classname: End-to-end
name: CSV API service resource not migrated if not adoptable
counts: 1
details:

count: 1
error: |4-

/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/csv_e2e_test.go:2985

	Error Trace:	csv_e2e_test.go:3064
	            				runner.go:113
	            				runner.go:64
	            				it_node.go:26
	            				spec.go:215
	            				spec.go:138
	            				spec_runner.go:200
	            				spec_runner.go:170
	            				spec_runner.go:66
	            				suite.go:62
	            				ginkgo_dsl.go:226
	            				ginkgo_dsl.go:214
	            				e2e_test.go:54
	Error:      	Received unexpected error:
	            	timed out waiting for the condition

/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/vendor/github.com/stretchr/testify/require/require.go:1005

meandurationsec: 305.124136
skippedtests:

classname: End-to-end
name: Subscription updates existing install plan
counts: 1
details: []
meandurationsec: 0.001395
classname: End-to-end
name: Subscriptions create required objects from Catalogs Given a Namespace when
a CatalogSource is created with a bundle that contains prometheus objects
creating a subscription using the CatalogSource should install the operator
successfully
counts: 1
details: []
meandurationsec: 6.515597
classname: End-to-end
name: Subscriptions create required objects from Catalogs Given a Namespace when
a CatalogSource is created with a bundle that contains prometheus objects
creating a subscription using the CatalogSource should have created the expected
prometheus objects
counts: 1
details: []
meandurationsec: 6.200362
classname: End-to-end
name: Catalog image update
counts: 1
details: []
meandurationsec: 0.00045
classname: End-to-end
name: Installing bundles with new object types when a bundle with a pdb, priorityclass,
and VPA object is installed should create the additional bundle objects
counts: 1
details: []
meandurationsec: 0

njhale · 2020-07-16T13:51:01Z

.github/workflows/e2e-tests.yml

@@ -14,5 +14,5 @@ jobs:
        if: ${{ failure() }}
        uses: actions/upload-artifact@v2
        with:
-          name: e2e-test-output-${{ github.sha }}-${{ github.run_id }}
+          name: e2e-test-output-${{ github.event.pull_request.head.sha }}-${{ github.run_id }}


is github.event.pull_request.head available when this workflow is run against master?

Do you mean when it is a commit instead of a PR?
We use PR for commenting purpose. Gather all the commits for a PR (by commit SHAs) and comment on the PR. If it is a commit to the master without PR, the result should be counted towards periodic report instead of PR comment. So it should be fine right? wdyt. Thanks

I am not entirely understanding the question here because this PR is opened against the master.

oh, I get it now, you are saying if we run this test without a PR what would happen. I have not tried it. I'll check

Bowenislandsong · 2020-07-16T14:14:06Z

/hold
need to experiment what head pr sha is if a test is triggered from a commit.

The env var GITHUB_SHA shows merge commit and is not related to worflow trigger unlike the discription on GITHUB. This fix changes the variable to look at the head commit aka. the commit that actually triggers the workflow.

Bowenislandsong · 2020-07-16T15:00:48Z

/hold cancel
https://github.com/Bowenislandsong/gmail-bot/runs/878105902?check_suite_focus=true
https://github.com/Bowenislandsong/gmail-bot/pull/2/checks

the new change is able to provide commit sha for both commit and PR changes.

njhale · 2020-07-16T15:06:10Z

Nice job!

/lgtm

Bowenislandsong · 2020-07-16T15:26:11Z

/retest

awgreene · 2020-07-16T15:51:46Z

/lgtm

ecordell · 2020-07-16T16:00:43Z

/approve

openshift-ci-robot · 2020-07-16T16:00:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bowenislandsong, ecordell

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ecordell]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2020-07-16T18:50:11Z

/retest

Please review the full test history for this PR and help us cut down flakes.

Bowenislandsong · 2020-07-16T20:01:31Z

/retest

openshift-bot · 2020-07-16T20:46:57Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-07-16T20:59:55Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-07-16T22:44:27Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-07-17T03:55:52Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-07-17T05:14:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

Bowenislandsong · 2020-07-18T06:30:32Z