fix(STONEINTG-524): fix issues

* fix issues according to comments
* switch to compare conclusion to decide if update is needed

Signed-off-by: Hongwei Liu <[email protected]>

controllers/integrationpipeline/integrationpipeline_adapter_test.go

@@ -60,7 +60,7 @@ func (r *MockStatusReporter) ReportStatus(client.Client, context.Context, *tekto
 	return r.ReportStatusError
 }
 
-func (r *MockStatusReporter) ReportStatusForSnapshot(client.Client, context.Context, *helpers.IntegrationLogger, *applicationapiv1alpha1.Snapshot, *[]gitops.IntegrationTestStatusDetail) error {
+func (r *MockStatusReporter) ReportStatusForSnapshot(client.Client, context.Context, *helpers.IntegrationLogger, *applicationapiv1alpha1.Snapshot) error {
 	r.Called = true
 	return r.ReportStatusError
 }

controllers/statusreport/statusreport_adapter.go

@@ -18,11 +18,8 @@ package statusreport
 
 import (
 	"context"
-	"encoding/json"
-	"fmt"
 
 	applicationapiv1alpha1 "github.com/redhat-appstudio/application-api/api/v1alpha1"
-	"github.com/redhat-appstudio/integration-service/gitops"
 	"github.com/redhat-appstudio/integration-service/helpers"
 	"github.com/redhat-appstudio/integration-service/status"
 
@@ -65,19 +62,7 @@ func (a *Adapter) EnsureSnapshotTestStatusReported() (controller.OperationResult
 	}
 
 	for _, reporter := range reporters {
-		integrationTestStatusDetails, err := getIntegrationScenarioTestStatusFromAnnotation(a.snapshot)
-		if err != nil {
-			a.logger.Error(err, "failed to get test status to snapshot annotations for snapshot",
-				"snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name)
-			return controller.RequeueWithError(err)
-		}
-		if integrationTestStatusDetails == nil {
-			a.logger.Info("no snapshot annotation test.appstudio.openshift.io/status defined for snapshot, no need to report integration test status",
-				"snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name)
-			return controller.ContinueProcessing()
-		}
-
-		if err := reporter.ReportStatusForSnapshot(a.client, a.context, &a.logger, a.snapshot, integrationTestStatusDetails); err != nil {
+		if err := reporter.ReportStatusForSnapshot(a.client, a.context, &a.logger, a.snapshot); err != nil {
 			a.logger.Error(err, "failed to report test status to github for snapshot",
 				"snapshot.Namespace", a.snapshot.Namespace, "snapshot.Name", a.snapshot.Name)
 			return controller.RequeueWithError(err)
@@ -86,19 +71,3 @@ func (a *Adapter) EnsureSnapshotTestStatusReported() (controller.OperationResult
 
 	return controller.ContinueProcessing()
 }
-
-// getIntegrationScenarioTestStatusFromAnnotation get the test status list for all integration test scenarios of snapshot
-// then Unmarshal to get structured data, return error if Unmarshal fails
-func getIntegrationScenarioTestStatusFromAnnotation(snapshot *applicationapiv1alpha1.Snapshot) (*[]gitops.IntegrationTestStatusDetail, error) {
-	statusAnnotation, ok := snapshot.GetAnnotations()[gitops.SnapshotTestsStatusAnnotation]
-	if !ok {
-		return nil, nil
-	}
-
-	integrationTestStatusDetails := &[]gitops.IntegrationTestStatusDetail{}
-	err := json.Unmarshal([]byte(statusAnnotation), integrationTestStatusDetails)
-	if err != nil {
-		return nil, fmt.Errorf("failed to load tests statuses from the scenario annotation: %w", err)
-	}
-	return integrationTestStatusDetails, nil
-}

controllers/statusreport/statusreport_adapter_test.go

@@ -18,6 +18,7 @@ package statusreport
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -51,7 +52,7 @@ func (r *MockStatusReporter) ReportStatus(client.Client, context.Context, *tekto
 	return r.ReportStatusError
 }
 
-func (r *MockStatusReporter) ReportStatusForSnapshot(client.Client, context.Context, *helpers.IntegrationLogger, *applicationapiv1alpha1.Snapshot, *[]gitops.IntegrationTestStatusDetail) error {
+func (r *MockStatusReporter) ReportStatusForSnapshot(client.Client, context.Context, *helpers.IntegrationLogger, *applicationapiv1alpha1.Snapshot) error {
 	r.Called = true
 	return r.ReportStatusError
 }
@@ -104,7 +105,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 					gitops.PipelineAsCodeInstallationIDAnnotation:   "123",
 					"build.appstudio.redhat.com/commit_sha":         "6c65b2fcaea3e1a0a92476c8b5dc89e92a85f025",
 					"appstudio.redhat.com/updateComponentOnSuccess": "false",
-					gitops.SnapshotTestsStatusAnnotation:            "[{\"ScenarioName\":\"scenario-1\",\"Status\":\"EnvironmentProvisionError\",\"StartTime\":\"2023-07-26T16:57:49+02:00\",\"CompletionTime\":\"2023-07-26T17:57:49+02:00\",\"Details\":\"Failed to find deploymentTargetClass with right provisioner for copy of existingEnvironment\"}]",
+					gitops.SnapshotTestsStatusAnnotation:            "[{\"scenario\":\"scenario-1\",\"status\":\"EnvironmentProvisionError\",\"startTime\":\"2023-07-26T16:57:49+02:00\",\"completionTime\":\"2023-07-26T17:57:49+02:00\",\"lastUpdateTime\":\"2023-08-26T17:57:49+02:00\",\"details\":\"Failed to find deploymentTargetClass with right provisioner for copy of existingEnvironment\"}]",
 				},
 			},
 			Spec: applicationapiv1alpha1.SnapshotSpec{
@@ -159,6 +160,7 @@ var _ = Describe("Snapshot Adapter", Ordered, func() {
 				},
 			})
 			result, err := adapter.EnsureSnapshotTestStatusReported()
+			fmt.Fprintf(GinkgoWriter, "-------err: %v\n", err)
 			Expect(!result.CancelRequest && err == nil).To(BeTrue())
 		})
 	})

git/github/github.go

@@ -359,13 +359,13 @@ func (c *Client) GetExistingCheckRun(checkRuns []*ghapi.CheckRun, newCheckRun *C
 }
 
 func (c *Client) IsUpdateNeeded(existingCheckRun *ghapi.CheckRun, newCheckRun *CheckRunAdapter) bool {
-	if newCheckRun.CompletionTime.After((*existingCheckRun.CompletedAt).Time) {
+	if newCheckRun.Conclusion != *existingCheckRun.Conclusion {
 		// We need to update the existing checkrun if the completionTime is after the gotten checkrun's CompletionTime
-		c.logger.Info("found CheckRun with a matching ExternalID and early CompletedAt, so need to update", "ExternalID", newCheckRun.ExternalID)
+		c.logger.Info("found CheckRun with a matching ExternalID and different Conclusion, so need to update", "ExternalID", newCheckRun.ExternalID)
 		return true
 	} else {
 		// We don't need to update the existing checkrun if the completionTime is not after the gotten checkrun's CompletionTime
-		c.logger.Info("found CheckRun with a matching ExternalID and CompletedAt, so no need to update", "ExternalID", newCheckRun.ExternalID)
+		c.logger.Info("found CheckRun with a matching ExternalID and Conclusion, so no need to update", "ExternalID", newCheckRun.ExternalID)
 		return false
 	}
 }

git/github/github_test.go

@@ -55,7 +55,8 @@ func (MockChecksService) ListCheckRunsForRef(
 ) (*ghapi.ListCheckRunsResults, *ghapi.Response, error) {
 	var id int64 = 20
 	var externalID string = "example-external-id"
-	checkRuns := []*ghapi.CheckRun{{ID: &id, ExternalID: &externalID, CompletedAt: &ghapi.Timestamp{Time: time.Now()}}}
+	conclusion := "failure"
+	checkRuns := []*ghapi.CheckRun{{ID: &id, ExternalID: &externalID, Conclusion: &conclusion}}
 	total := len(checkRuns)
 	return &ghapi.ListCheckRunsResults{Total: &total, CheckRuns: checkRuns}, nil, nil
 }
@@ -66,7 +67,8 @@ func (MockChecksService) GetAllCheckRunsForRef(
 ) ([]*ghapi.CheckRun, error) {
 	var id int64 = 20
 	var externalID string = "example-external-id"
-	checkRuns := []*ghapi.CheckRun{{ID: &id, ExternalID: &externalID, CompletedAt: &ghapi.Timestamp{Time: time.Now()}}}
+	conclusion := "failure"
+	checkRuns := []*ghapi.CheckRun{{ID: &id, ExternalID: &externalID, Conclusion: &conclusion}}
 	return checkRuns, nil
 }
 
@@ -239,7 +241,7 @@ var _ = Describe("Client", func() {
 			Summary:        "example-summary",
 			Text:           "example-text",
 			StartTime:      time.Now(),
-			CompletionTime: time.Now().Add(5 * time.Minute),
+			CompletionTime: time.Now(),
 		}
 
 		allCheckRuns, err := client.GetAllCheckRunsForRef(context.TODO(), "", "", "", 1)
@@ -256,12 +258,12 @@ var _ = Describe("Client", func() {
 			Repository:     "example-repo",
 			SHA:            "abcdef1",
 			ExternalID:     "example-external-id",
-			Conclusion:     "success",
+			Conclusion:     "failure",
 			Title:          "example-title",
 			Summary:        "example-summary",
 			Text:           "example-text",
 			StartTime:      time.Now(),
-			CompletionTime: existingCheckRun.CompletedAt.Time,
+			CompletionTime: time.Now(),
 		}
 		Expect(client.IsUpdateNeeded(existingCheckRun, checkRunAdapter)).To(BeFalse())
 	})

gitops/snapshot.go

@@ -119,17 +119,17 @@ const (
 	//AppStudioIntegrationStatusFinished is the reason that's set when the AppStudio tests finish.
 	AppStudioIntegrationStatusFinished = "Finished"
 
-	//IntegrationTestStatusQueuedGithub is the status reported to github when integration test is in a queue
-	IntegrationTestStatusQueuedGithub = "queued"
-
-	//IntegrationTestStatusInProgressGithub is the status reported to github when integration test is in progress
-	IntegrationTestStatusInProgressGithub = "in_progress"
+	//IntegrationTestStatusPendingGithub is the status reported to github when integration test is in a queue
+	IntegrationTestStatusPendingGithub = "pending"
 
 	//IntegrationTestStatusSuccessGithub is the status reported to github when integration test succeed
 	IntegrationTestStatusSuccessGithub = "success"
 
 	//IntegrationTestStatusFailureGithub is the status reported to github when integration test fail
 	IntegrationTestStatusFailureGithub = "failure"
+
+	//IntegrationTestStatusErrorGithub is the status reported to github when integration test experience error
+	IntegrationTestStatusErrorGithub = "error"
 )
 
 // IntegrationTestScenario test runs status

gitops/snapshot_predicate_test.go

@@ -99,7 +99,7 @@ var _ = Describe("Predicates", Ordered, func() {
 					gitops.SnapshotComponentLabel: componentName,
 				},
 				Annotations: map[string]string{
-					gitops.SnapshotTestsStatusAnnotation: "[{\"ScenarioName\":\"scenario-1\",\"Status\":\"EnvironmentProvisionError\",\"StartTime\":\"2023-07-26T16:57:49+02:00\",\"CompletionTime\":\"2023-07-26T17:57:49+02:00\",\"Details\":\"Failed to find deploymentTargetClass with right provisioner for copy of existingEnvironment\"}]",
+					gitops.SnapshotTestsStatusAnnotation: "[{\"scenario\":\"scenario-1\",\"status\":\"EnvironmentProvisionError\",\"startTime\":\"2023-07-26T16:57:49+02:00\",\"sompletionTime\":\"2023-07-26T17:57:49+02:00\",\"details\":\"Failed to find deploymentTargetClass with right provisioner for copy of existingEnvironment\"}]",
 				},
 			},
 			Spec: applicationapiv1alpha1.SnapshotSpec{
@@ -122,7 +122,7 @@ var _ = Describe("Predicates", Ordered, func() {
 					gitops.SnapshotComponentLabel: componentName,
 				},
 				Annotations: map[string]string{
-					gitops.SnapshotTestsStatusAnnotation: "[{\"ScenarioName\":\"scenario-1\",\"Status\":\"TestPassed\",\"StartTime\":\"2023-07-26T16:57:49+02:00\",\"CompletionTime\":\"2023-07-26T17:57:49+02:00\",\"Details\": \"test pass\"}]",
+					gitops.SnapshotTestsStatusAnnotation: "[{\"scenario\":\"scenario-1\",\"status\":\"TestPassed\",\"startTime\":\"2023-07-26T16:57:49+02:00\",\"completionTime\":\"2023-07-26T17:57:49+02:00\",\"details\": \"test pass\"}]",
 				},
 			},
 			Spec: applicationapiv1alpha1.SnapshotSpec{