Run integration test with GCS mock #55

mariiatuzovska · 2024-12-09T17:50:23Z

This PR introduces integration tests to ensure the proper functioning of key components with fake-gcs-server as a dependency.
These tests provide a more comprehensive validation by simulating interactions with a Google Cloud Storage-like environment.

Key Changes:

Added integration tests that rely on fake-gcs-server.
Updated documentation with instructions on running the tests locally using Docker.
Modify CI job for integration tests.

amanjpro · 2024-12-10T19:47:58Z

README.md

+Use the following command to run the integration tests:
+
+bash```
+STORAGE_EMULATOR_HOST=http://0.0.0.0:4443 go test ./pkg/cmd/cli/... -run="TestCmd"


Technically all our tests are more or less a form of it-test, so I'll remove this part in the command: 'run="TestCmd"'

amanjpro · 2024-12-10T19:51:05Z

pkg/cmd/cli/run_it_test.go

+	// test parameters for each test
+	cleanroomName               string
+	expireTime                  time.Time
+	emailsSource                []string
+	tmpDir                      string
+	salt                        string
+	publisherPairKey            *pair.PrivateKey
+	advertiserInputFilePath     string
+	advertiserKeyConfigFilePath string
+	publisherPAIRIDsFolderPath  string
+	advertiserOutputFolderPath  string


Should these be part of a different struct? to me these do not belong to the "test-suite" struct

amanjpro · 2024-12-10T20:03:15Z

pkg/cmd/cli/run_it_test.go

+	// STORAGE_EMULATOR_HOST is required to run this test
+	bucketURL := os.Getenv("STORAGE_EMULATOR_HOST")
+	if bucketURL == "" {
+		t.Skip("STORAGE_EMULATOR_HOST is required to run this test")


Instead of skipping, this test should fail IMO

amanjpro · 2024-12-10T20:04:39Z

pkg/cmd/cli/run_it_test.go

+	shaEncoder := sha256.New()
+	for i := range genEmailsSourceNumber {
+		shaEncoder.Write([]byte(fmt.Sprintf("%d@gmail.com", i)))
+		hem := shaEncoder.Sum(nil)
+		s.emailsSource[i] = fmt.Sprintf("%x", hem)
+	}


Didn't you already have a helper function to generate emails?

yes, I have one in pair package. I cannot import it from _test.go file.

amanjpro · 2024-12-10T20:07:22Z

pkg/cmd/cli/run_it_test.go

+	}
+}
+
+func (s *cmdTestSuite) requireGenPublisherTripleEncryptedData() {


Why do you have this? publisher-triple-encrypted data is generated by the CLI, so you should not be creating it yourself. You probably need requireGenAdvertiserTripleEncryptedData (which is created by the publisher)

naming mistake

it actually writes into advertiser triple encrypted folder

amanjpro · 2024-12-10T20:13:53Z

pkg/cmd/cli/run_it_test.go

+				s.requireGenPublisherTripleEncryptedData()
+			}
+
+			nextPuplisherState = v1.Cleanroom_Participant_SUCCEEDED


In the cleanroom service side, this will only happen if the CLI has pushed triple-encrypted-publisher data and called the advance-state the second time

yes, it is called two times. I change variables "nextPuplisherState" and "nextAdvertiserState".

amanjpro · 2024-12-10T20:15:25Z

pkg/cmd/cli/run_it_test.go

+
+			if cleanroom.Participants[0].State == v1.Cleanroom_Participant_DATA_TRANSFORMED {
+				// add publisher triple encrypted data on this step
+				s.requireGenPublisherTripleEncryptedData()


I am not understanding this, why do we do it here? or the function name should requireGenAdvertiserTripleEncryptedData?

I need it for match: https://github.com/Optable/optable-pair-cli/blob/main/pkg/cmd/cli/run.go#L200

amanjpro · 2024-12-10T20:24:45Z

Oh, you will also need to pass the two options:

--output=...
----save-publisher-encrypted-data-locally=...

So it does check the actual match, and uses the locally stored publisher data. THat will make the coverage data almost 100%

mariiatuzovska · 2024-12-10T20:34:45Z

Oh, you will also need to pass the two options:

--output=... ----save-publisher-encrypted-data-locally=...

So it does check the actual match, and uses the locally stored publisher data. THat will make the coverage data almost 100%

It is already done and checked here: https://github.com/Optable/optable-pair-cli/pull/55/files#diff-68235dad6501edbd1da912df01435e618628c88dfdf962f4d4facb483fc8c093R325-R326

README.md

juanli16 · 2024-12-11T15:36:53Z

README.md

+Use the following command to run the integration tests:
+
+bash```
+STORAGE_EMULATOR_HOST=http://0.0.0.0:4443 go test ./...


@amanjpro suggested to add

I think what he says is in the README, we no longer this detail, because we already have the Make target, and I agree

We probably just need to tell you cannot run the tests under a windows machine

juanli16 · 2024-12-11T15:41:34Z

pkg/bucket/bucket.go

@@ -56,6 +57,8 @@ type (
 	Option func(*bucketOptions)
 )

+var HTTPClient = &http.Client{}


don't set global variable, pass it instead?

the other way is to pass it in the cli context, what do you suggest?

or as an optional value, and the constructor can check if it is nil or not, and sets it accordingly?

pkg/cmd/cli/run_it_test.go

juanli16 · 2024-12-11T15:47:50Z

pkg/cmd/cli/run_it_test.go

+
+func (s *cmdTestSuite) requireGenAdvertiserTripleEncryptedData() {
+	s.T().Helper()
+	ctx := context.Background()


juanli16 · 2024-12-11T15:50:19Z

pkg/cmd/cli/run_it_test.go

+		s.Require().NoError(err, "must close GCS writer")
+	}()
+	tripleEncryptedCsvWriter := csv.NewWriter(tripleEncryptedWriter)
+	defer tripleEncryptedCsvWriter.Flush()


No need to defer this, and you call writer.Error() to check write errors.

Flush writes any buffered data to the underlying io.Writer. To check if an error occurred during Flush, call Writer.Error.

juanli16 · 2024-12-11T15:50:41Z

pkg/cmd/cli/run_it_test.go

+
+func (s *cmdTestSuite) requireLocalContentEqualToGCSContent(localFolder, gcsFolder string) {
+	s.T().Helper()
+	ctx := context.Background()


Suggested change

ctx := context.Background()

juanli16 · 2024-12-11T15:52:09Z

pkg/cmd/cli/run_it_test.go

+	}()
+
+	w := csv.NewWriter(tmpInputFile)
+	defer w.Flush()


check w.Error()

…nstead

Co-authored-by: Justin Li <justin.li@optable.co>

mariiatuzovska added 3 commits December 9, 2024 12:49

WIP

4532a5c

get right obj path

4db15e5

fix test and add doc

cd4ce5f

mariiatuzovska changed the title ~~WIP: Run integration test with GCS mock~~ Run integration test with GCS mock Dec 10, 2024

mariiatuzovska and others added 9 commits December 10, 2024 12:25

Merge branch 'main' into mock-gcs

20cc897

add CI (#56)

46257b5

adress CI comments

729b960

fix path

1874ea8

rm -backend memory flaag

0c13a37

fix comment

f9eb7ef

fix defer

595d0e5

trim this

392c2a9

add make commands to ru local tests

ebf8f9a

amanjpro reviewed Dec 10, 2024

View reviewed changes

mariiatuzovska and others added 5 commits December 10, 2024 15:36

add params and fix readme

978b7e9

add coverage

36d7d03

rm coverage

a570435

scheme

8763781

Fix make file

e5f429c

juanli16 reviewed Dec 11, 2024

View reviewed changes

mariiatuzovska and others added 8 commits December 11, 2024 11:05

Http clien option owerwrites source client option: use option slice i…

444d192

…nstead

Update README.md

dd8f08f

Co-authored-by: Justin Li <justin.li@optable.co>

Update README.md

c82374d

Co-authored-by: Justin Li <justin.li@optable.co>

Update pkg/cmd/cli/run_it_test.go

5f3b8e3

Co-authored-by: Justin Li <justin.li@optable.co>

rm ctx

7d9f8ae

chec err for csv writer

0c45a03

fix readme

6991bdb

rm it test section from the readme

364c7ac

rm it test from readme #2

33fc9a2

amanjpro approved these changes Dec 11, 2024

View reviewed changes

fix tmp dir in TestKeyCreate

b38580a

mariiatuzovska merged commit 45fd713 into main Dec 11, 2024
3 checks passed

mariiatuzovska deleted the mock-gcs branch December 11, 2024 19:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run integration test with GCS mock #55

Run integration test with GCS mock #55

mariiatuzovska commented Dec 9, 2024 •

edited

Loading

amanjpro Dec 10, 2024

amanjpro Dec 10, 2024

amanjpro Dec 10, 2024

amanjpro Dec 10, 2024

mariiatuzovska Dec 10, 2024 •

edited

Loading

amanjpro Dec 10, 2024

mariiatuzovska Dec 10, 2024

mariiatuzovska Dec 10, 2024

amanjpro Dec 10, 2024

mariiatuzovska Dec 10, 2024

amanjpro Dec 10, 2024

mariiatuzovska Dec 10, 2024

amanjpro commented Dec 10, 2024

mariiatuzovska commented Dec 10, 2024 •

edited

Loading

juanli16 Dec 11, 2024

mariiatuzovska Dec 11, 2024

amanjpro Dec 11, 2024

juanli16 Dec 11, 2024

mariiatuzovska Dec 11, 2024

amanjpro Dec 11, 2024

juanli16 Dec 11, 2024

juanli16 Dec 11, 2024

juanli16 Dec 11, 2024

juanli16 Dec 11, 2024

Run integration test with GCS mock #55

Run integration test with GCS mock #55

Conversation

mariiatuzovska commented Dec 9, 2024 • edited Loading

Key Changes:

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

mariiatuzovska Dec 10, 2024 • edited Loading

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

amanjpro commented Dec 10, 2024

mariiatuzovska commented Dec 10, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mariiatuzovska commented Dec 9, 2024 •

edited

Loading

mariiatuzovska Dec 10, 2024 •

edited

Loading

mariiatuzovska commented Dec 10, 2024 •

edited

Loading