test: Added Unit tests for stream.go file's methods by supersanchayrx · Pull Request #980 · datazip-inc/olake

supersanchayrx · 2026-06-09T15:55:09Z

Description

Added comprehensive unit tests for stream.go file.

Fixes #661

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

All tests verified via go test ./types -v -run "TestStream_"

Screenshots or Recordings

stream.file.tests.mp4

Video Content :

All unit test cases pass.

sync.tests.mp4

Video Content:

The streams.go interaction with other files still work properly as the sync shows expected behaviour.

Documentation

N/A (bug fix, refactor, or test changes only)

Related PR's (If Any):

#680

added test for NewStream(), WithSyncMode(), WithPrimaryKey(), WithCursorField(), WithSchema(), Wrap(). left for WithUpsertField(), UnmarshalJson(), StreamsToMap(), LogCatalog(), ID().

supersanchayrx · 2026-06-09T15:57:20Z

Also very grateful to @Beeram12 for his contributions. These tests were added on top of his work done from pr #680

CLAassistant · 2026-06-09T16:03:02Z

All committers have signed the CLA.

nayanj98 · 2026-06-10T06:05:50Z

@supersanchayrx Thanks a lot! Will assign a reviewer for this PR soon.

nayanj98 · 2026-06-11T13:17:59Z

@supersanchayrx Assigning @saksham-datazip to review your PR

Signed-off-by: Sanchay <grindinforvalore@gmail.com>

saksham-datazip

Sorry for the late review and please make the respective changes.

saksham-datazip · 2026-06-22T11:07:09Z

+	})
+}
+
+func TestStream_UnmarshalJSON(t *testing.T) {


Please try to follow patterns as the way we followed in other files by creating a struct and also make the same changes in function below as well.

Could you please use the same test case structure that's used in the other tests as well? It would help maintain consistency across the test suite for All the functions.Like this:-

tests := []struct { testName string syncIndex int }{

Instead of directly using t.Run

done
it now uses this kind of tests struct pattern only. it has per field expected values to cross check against. all of the test cases are traversed through loop. Made sure this pattern remains consistent with other methods as well.

supersanchayrx · 2026-06-24T18:16:50Z

Hey sure I'll fix these asap thanks for the review

saksham-datazip · 2026-06-29T06:12:08Z

@supersanchayrx Are you still working on this unit test?

supersanchayrx · 2026-06-29T13:43:45Z

Hey @saksham-datazip sorry for the delay. Yup I implemented your suggestions and fixed the code wherever needed. Please review it once more. Thanks!

saksham-datazip

I've addressed the previous comments on my end, but it looks like a few comments are still pending from your side. Could you please take a look at those as well? If you have any questions or need any clarification, feel free to ask in the comments.

saksham-datazip · 2026-07-02T13:14:05Z

+			asserts.Equal(tt.expectedStream.Name, stream.Name)
+			asserts.Equal(tt.expectedStream.Namespace, stream.Namespace)
+			asserts.NotNil(stream.SupportedSyncModes, "SupportedSyncModes should be initialized")
+			asserts.NotNil(stream.SourceDefinedPrimaryKey, "SourceDefinedPrimaryKey should be initialized")
+			asserts.NotNil(stream.AvailableCursorFields, "AvailableCursorFields should be initialized")
+			asserts.NotNil(stream.Schema, "Schema should be initialized")
+			asserts.Equal(tt.expectedStream.DestinationDatabase, stream.DestinationDatabase)
+			asserts.Equal(tt.expectedStream.DestinationTable, stream.DestinationTable)


instead of this much checks you should use asserts.Equal(tt.expectedStream, stream)
and apart from this the streams which we are getting as output are different from expected so consider using complete structure, Like this :-

expectedStream: &Stream{ Name: "User-Orders.v2", Namespace: "My.Schema", Schema: NewTypeSchema(), SupportedSyncModes: NewSet[SyncMode](), SourceDefinedPrimaryKey: NewSet[string](), AvailableCursorFields: NewSet[string](), DestinationDatabase: ":my_schema", DestinationTable: "user_orders_v2", },

Note: This is a dummy code

Done! thanks for the dummy code 👍

saksham-datazip · 2026-07-02T13:44:03Z

+			stream := NewStream("users", "public", nil)
+			returnedStream := stream.WithPrimaryKey(tt.keys...)
+


returnedStream was previously named outputStream. Could we make the naming consistent? Also, if there are any other variable names that could be made more consistent throughout the test for better readability, it would be nice to update those as well.

I have changed this and also tried to keep consistent naming across file

saksham-datazip · 2026-07-02T13:45:37Z

+			testName:     "multiple keys",
+			keys:         []string{"id", "user_uuid"},
+			expectedKeys: []string{"id", "user_uuid"},
+		},
+		{
+			testName:     "composite key",
+			keys:         []string{"tenant_id", "user_id", "order_id"},
+			expectedKeys: []string{"tenant_id", "user_id", "order_id"},
+		},


multiple keys test and composite keys tests are kind of same just with different number of keys and due to their naming ig this testName "keys with underscores" is redundant.

Yes i've removed the redundant test cases.

saksham-datazip · 2026-07-02T14:03:54Z

+		testName       string
+		fields         []string
+		expectedFields []string


Can you change this fields name with the function input name also check for other variables.

changed 'fields' -> 'columns' to match the function input var name

saksham-datazip · 2026-07-02T14:05:48Z

+		{
+			testName:       "single field",
+			fields:         []string{"updated_at"},
+			expectedFields: []string{"updated_at"},
+		},
+		{
+			testName:       "multiple fields",
+			fields:         []string{"updated_at", "inserted_at"},
+			expectedFields: []string{"updated_at", "inserted_at"},
+		},
+		{
+			testName:       "timestamp fields",
+			fields:         []string{"created_at", "updated_at", "deleted_at"},
+			expectedFields: []string{"created_at", "updated_at", "deleted_at"},
+		},
+		{
+			testName:       "duplicate fields",
+			fields:         []string{"updated_at", "updated_at", "inserted_at"},
+			expectedFields: []string{"updated_at", "inserted_at"},
+		},
+		{
+			testName:       "empty fields",
+			fields:         []string{},
+			expectedFields: []string{},
+		},
+		{
+			testName:       "fields with underscores",


Also Can you please confirm these all checks seems to be redundant ?

yes removed the redundant check

saksham-datazip · 2026-07-02T14:14:13Z

+	//test for merging two datatypes on subsequent upsert calls with same names
+	t.Run("Multiple datatypes test", func(t *testing.T) {
+		asserts := assert.New(t)
+		stream := NewStream("phones", "seller", nil)
+
+		stream.UpsertField("codename", Int64, false, false)
+		stream.UpsertField("codename", String, true, true)
+


Can we write this testcase like others no need for a new t.Run ?

I can do it but I purposely kept it separate as it needs to check whether 2 usertfields on 1 col actually gives merged types. Making this into the table tests would reduce readibilty in my opinion and would also look a bit odd but nonetheless I can make it happen. Do let me know I'll make that change quick

saksham-datazip · 2026-07-02T18:31:27Z

+		{
+			testName:  "wrap with large index",
+			syncIndex: 100,
+		},


Look into those tc as this function is input independent 1-2 basic testcases is more than enough

yeah removed trivial ones. Just kept the -ve and 0

saksham-datazip · 2026-07-02T18:31:56Z

+	for _, tt := range tests {
+		t.Run(tt.testName, func(t *testing.T) {
+			stream := NewStream("users", "public", nil)
+			configuredStream := stream.Wrap(tt.syncIndex)


here also try to make this name configuredStream consistent as told earlier

yes followed consistency for all variables in general throughout the file

saksham-datazip · 2026-07-02T18:32:20Z

+			assert.NotNil(t, configuredStream, "Should return a configuredStream")
+			assert.Same(t, stream, configuredStream.Stream, "Should wrap the exact same stream instance")


Can we use equal instead of those two lines ?

yes used equal directly of 2 individual checks.

saksham-datazip · 2026-07-02T18:34:02Z

+		})
+	}
+}
+


For the function after line 538 i am expecting you to make changes if possible else a reason why we cant follow olake pattern.

done. Sorry for not resolving this in last review
it now uses this kind of tests struct pattern only as you suggested in other comment. it has per field expected values to cross check against. all of the test cases are traversed through loop. Made sure this pattern remains consistent with other methods as well.

saksham-datazip · 2026-07-08T10:31:18Z

@supersanchayrx Hey! Are you still working on this issue? If so, please let us know. Otherwise, we'll have to close this PR due to inactivity.

supersanchayrx · 2026-07-08T12:26:51Z

hey yes sorry for the delay. I'm pushing an update asap

supersanchayrx · 2026-07-12T20:35:06Z

Please have a review once more

supersanchayrx added 4 commits June 9, 2026 01:52

Added unit tests for stream.go datazip-inc#661

1eb62d6

added test for NewStream(), WithSyncMode(), WithPrimaryKey(), WithCursorField(), WithSchema(), Wrap(). left for WithUpsertField(), UnmarshalJson(), StreamsToMap(), LogCatalog(), ID().

Added Remaining tests for stream.go

9a61b83

Merge branch 'staging' into unit-test-stream.go

7746b07

Merge branch 'datazip-inc:master' into unit-test-stream.go

82e7967

supersanchayrx changed the title ~~Added Unit tests for stream.go file's methods~~ test : Added Unit tests for stream.go file's methods Jun 9, 2026

nayanj98 requested a review from saksham-datazip June 11, 2026 13:18

Merge branch 'staging' into unit-test-stream.go

8e4053b

saksham-datazip temporarily deployed to integration_tests June 22, 2026 07:16 — with GitHub Actions Inactive

supersanchayrx changed the title ~~test : Added Unit tests for stream.go file's methods~~ test: Added Unit tests for stream.go file's methods Jun 22, 2026

Fixed whitespace issues

68d0a70

Signed-off-by: Sanchay <grindinforvalore@gmail.com>

saksham-datazip reviewed Jun 22, 2026

View reviewed changes

supersanchayrx had a problem deploying to integration_tests June 22, 2026 11:31 — with GitHub Actions Failure

supersanchayrx temporarily deployed to integration_tests June 22, 2026 11:31 — with GitHub Actions Inactive

Merge branch 'staging' into unit-test-stream.go

58dce24

Implemented Review suggestions

cccbbc3

supersanchayrx added 2 commits June 29, 2026 19:17

removed whitespaces

bbaff44

Merge branch 'staging' into unit-test-stream.go

614a97b

supersanchayrx temporarily deployed to integration_tests July 2, 2026 12:37 — with GitHub Actions Inactive

saksham-datazip reviewed Jul 2, 2026

View reviewed changes

Merge branch 'staging' into unit-test-stream.go

35a8624

saksham-datazip requested a deployment to integration_tests July 3, 2026 09:06 — with GitHub Actions Waiting

saksham-datazip temporarily deployed to integration_tests July 3, 2026 09:06 — with GitHub Actions Inactive

Changes to stream_test file as per review

e523e46

Merge branch 'staging' into unit-test-stream.go

f067db6

supersanchayrx requested a deployment to integration_tests July 13, 2026 08:58 — with GitHub Actions Waiting

Merge branch 'staging' into unit-test-stream.go

d199b33

		stream := NewStream("users", "public", nil)
		returnedStream := stream.WithPrimaryKey(tt.keys...)

		assert.NotNil(t, configuredStream, "Should return a configuredStream")
		assert.Same(t, stream, configuredStream.Stream, "Should wrap the exact same stream instance")

Uh oh!

Conversation

supersanchayrx commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Screenshots or Recordings

Video Content :

Video Content:

Documentation

Related PR's (If Any):

Uh oh!

supersanchayrx commented Jun 9, 2026

Uh oh!

CLAassistant commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nayanj98 commented Jun 10, 2026

Uh oh!

nayanj98 commented Jun 11, 2026

Uh oh!

saksham-datazip left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saksham-datazip Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

supersanchayrx commented Jun 24, 2026

Uh oh!

saksham-datazip commented Jun 29, 2026

Uh oh!

supersanchayrx commented Jun 29, 2026

Uh oh!

saksham-datazip left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

supersanchayrx commented Jun 9, 2026 •

edited

Loading

CLAassistant commented Jun 9, 2026 •

edited

Loading

saksham-datazip Jul 2, 2026 •

edited

Loading

supersanchayrx Jul 12, 2026 •

edited

Loading