Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for Sync & Discover mode #7

Merged
merged 7 commits into from
Jul 8, 2022
Merged

Add tests for Sync & Discover mode #7

merged 7 commits into from
Jul 8, 2022

Conversation

Phanatic
Copy link
Contributor

@Phanatic Phanatic commented Jul 5, 2022

Adds tests for the Sync and Discover mode in the CLI

=== RUN   TestSync_CanFilterSchema
--- PASS: TestSync_CanFilterSchema (0.00s)
=== RUN   TestSync_CanStartFromEmptyState
--- PASS: TestSync_CanStartFromEmptyState (0.00s)
=== RUN   TestSync_PrintsStreamSchema
--- PASS: TestSync_PrintsStreamSchema (0.00s)
=== RUN   TestSync_PrintsStreamState
--- PASS: TestSync_PrintsStreamState (0.00s)
=== RUN   TestSync_UsesStateIfIncrementalSyncRequested
--- PASS: TestSync_UsesStateIfIncrementalSyncRequested (0.00s)
=== RUN   TestSync_PrintsOldStateIfNoNewStateFound
--- PASS: TestSync_PrintsOldStateIfNoNewStateFound (0.00s)
=== RUN   TestSync_PrintsNewStateIfFound
--- PASS: TestSync_PrintsNewStateIfFound (0.00s)

=== RUN   TestDiscover_CanFailIfCredentialsInvalid
--- PASS: TestDiscover_CanFailIfCredentialsInvalid (0.00s)
=== RUN   TestDiscover_CanFailIfCannotQuery
--- PASS: TestDiscover_CanFailIfCannotQuery (0.00s)
=== RUN   TestDiscover_SchemaHasPrimaryKeys
--- PASS: TestDiscover_SchemaHasPrimaryKeys (0.00s)
=== RUN   TestDiscover_SchemaHasCursorProperties
--- PASS: TestDiscover_SchemaHasCursorProperties (0.00s)
=== RUN   TestDiscover_SchemaHasValidMetadata
--- PASS: TestDiscover_SchemaHasValidMetadata (0.00s)

@Phanatic Phanatic changed the title [wip] Add tests for Discover mode Add tests for Sync & Discover mode Jul 5, 2022
@Phanatic Phanatic marked this pull request as ready for review July 5, 2022 21:51
@Phanatic Phanatic requested a review from mdlayher July 5, 2022 21:51
cmd/internal/discover_test.go Show resolved Hide resolved
emp := c.Streams[0]
mm := emp.Metadata.GetPropertyMap()
assert.Equal(t, "automatic", mm["emp_no"].Metadata.Inclusion, "key properties should be auto-included")
assert.Equal(t, mm["emp_no"].Metadata.BreadCrumb, []string{"properties", "emp_no"})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once you start to get into big nested comparisons like this, it gets easy to gloss over the details. It may be easier to compare a full "want" map versus the "got" result from GetPropertyMap so that the test libraries can spit out a nice diff of the whole thing rather than making assertions on each key/value pair.

if err != nil {
return err
}

if newCursor != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that it's possible for Read to return nil, nil above? Is that an expected behavior? Generally I'd expect some sort of sentinel value or error instead.

cmd/internal/sync_test.go Outdated Show resolved Hide resolved
Phani Raj and others added 2 commits July 8, 2022 11:13
Co-authored-by: Matt Layher <mdlayher@gmail.com>
@Phanatic Phanatic merged commit 15f6e36 into main Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants