Repository: google/exposure-notifications-server
Pull request #1163 (2020-11-12 18:24:19)

Incremental e2e tests: use latest service for running tests
Just discovered that incremental e2e test had been creating images and migrating db, but didn't really deploy new services, so the test had been testing the old services from last month. This PR should fix it

Ported from  https://github.com/google/exposure-notifications-verification-server/pull/1067

google-oss-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/google/exposure-notifications-server/pull/1163#" title="Author self-approved">chaodaiG</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=google%2Fexposure-notifications-server).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/google/exposure-notifications-server/blob/main/OWNERS)~~ [chaodaiG]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
chaodaiG: /cc @sethvargo 

Patch:
diff --git a/internal/publish/model/exposure_model.go b/internal/publish/model/exposure_model.go
index 2f5d682..ff92b94 100644
--- a/internal/publish/model/exposure_model.go
+++ b/internal/publish/model/exposure_model.go
@@ -173,6 +173,10 @@ func (e *Exposure) Revise(in *Exposure) (bool, error) {
 	if e.ReportType == in.ReportType {
 		return false, nil
 	}
+	// Key is being published again, but has already been revised to target report type.
+	if e.RevisedAt != nil && e.RevisedReportType != nil && *e.RevisedReportType == in.ReportType {
+		return false, nil
+	}
 	if !e.LocalProvenance {
 		nonLocalOK := false
 		if e.ExportImportID != nil {


Test patch:
diff --git a/internal/publish/database/exposure_test.go b/internal/publish/database/exposure_test.go
index 5cdac02..c01b31d 100644
--- a/internal/publish/database/exposure_test.go
+++ b/internal/publish/database/exposure_test.go
@@ -577,6 +577,9 @@ func TestReviseExposures(t *testing.T) {
 			t.Errorf("expected %d to be %d: %#v", got, want, reviseResp)
 		}
 
+		// Change report, type, attempt to revise again.
+		exposure.ReportType = verifyapi.ReportTypeNegative
+
 		// Attempt to revise the same key again - this should fail.
 		if _, err := pubDB.InsertAndReviseExposures(ctx, &InsertAndReviseExposuresRequest{
 			Incoming:     []*model.Exposure{exposure},
diff --git a/internal/publish/model/exposure_model_test.go b/internal/publish/model/exposure_model_test.go
index 3e6e920..46a6252 100644
--- a/internal/publish/model/exposure_model_test.go
+++ b/internal/publish/model/exposure_model_test.go
@@ -1536,6 +1536,42 @@ func TestExposureReview(t *testing.T) {
 			needsRevision: true,
 			err:           "",
 		},
+		{
+			name: "double_revise",
+			previous: &Exposure{
+				ReportType:            verifyapi.ReportTypeClinical,
+				LocalProvenance:       true,
+				HealthAuthorityID:     int64Ptr(2),
+				Regions:               []string{"US", "CA"},
+				TransmissionRisk:      4,
+				CreatedAt:             createdAt,
+				DaysSinceSymptomOnset: int32Ptr(-1),
+				RevisedAt:             &createdAt,
+				RevisedReportType:     stringPtr(verifyapi.ReportTypeConfirmed),
+			},
+			incoming: &Exposure{
+				ReportType:            verifyapi.ReportTypeConfirmed,
+				HealthAuthorityID:     int64Ptr(3),
+				Regions:               []string{"MX"},
+				TransmissionRisk:      5,
+				CreatedAt:             revisedAt,
+				DaysSinceSymptomOnset: int32Ptr(0),
+			},
+			want: &Exposure{
+				ReportType:                   verifyapi.ReportTypeClinical,
+				LocalProvenance:              true,
+				HealthAuthorityID:            int64Ptr(3),
+				Regions:                      []string{"US", "CA", "MX"},
+				TransmissionRisk:             4,
+				CreatedAt:                    createdAt,
+				DaysSinceSymptomOnset:        int32Ptr(-1),
+				RevisedReportType:            stringPtr(verifyapi.ReportTypeConfirmed),
+				RevisedAt:                    &revisedAt,
+				RevisedDaysSinceSymptomOnset: int32Ptr(0),
+				RevisedTransmissionRisk:      intPtr(5),
+			},
+			needsRevision: false,
+		},
 	}
 
 	for _, tc := range cases {
diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go
index 8b6a6f5..0f80773 100644
--- a/internal/publish/publish_test.go
+++ b/internal/publish/publish_test.go
@@ -862,9 +862,9 @@ func TestKeyRevision(t *testing.T) {
 					t.Fatal(err)
 				}
 
-				// Revise them
+				// Revise them to negative - test will try to revise to positive.
 				for _, key := range incoming {
-					key.ReportType = verifyapi.ReportTypeConfirmed
+					key.ReportType = verifyapi.ReportTypeNegative
 				}
 				if _, err := pubDB.InsertAndReviseExposures(ctx, &pubdb.InsertAndReviseExposuresRequest{
 					Incoming: incoming,
diff --git a/scripts/ci-e2e-test.sh b/scripts/ci-e2e-test.sh
index 48d0e97..835af73 100755
--- a/scripts/ci-e2e-test.sh
+++ b/scripts/ci-e2e-test.sh
@@ -43,6 +43,10 @@ function incremental() {
   fi
 
   ${ROOT}/scripts/ci-terraform.sh init
+
+  # Running "${ROOT}/scripts/deploy" would fail when a service doesn't not exist,
+  # e.g. a new cloud run service introduced, due to `--no-traffic` not allowed for
+  # newly created service. So let terraform create the service first.
   pushd "${ROOT}/terraform-e2e-ci" >/dev/null 2>&1
   # Force terraform to redo build and migrate. This step might fail when the previous e2e run failed,
   # as build or migrate was destroyed but not restalled in terraform state.
@@ -61,6 +65,10 @@ function incremental() {
   export E2E_DB_PASSWORD="secret://${E2E_DB_PASSWORD}"
   export E2E_DB_SSLMODE=disable
 
+  ${ROOT}/scripts/build
+  ${ROOT}/scripts/deploy
+  ${ROOT}/scripts/promote
+
   run_e2e_test
 }