Skip to content

Conversation

@bplunkett-stripe
Copy link
Collaborator

@bplunkett-stripe bplunkett-stripe commented Feb 1, 2024

Description

When using temp db factories (which have metadata tables), we need to tell the schema fetcher to ignore those tables.

This also technically switches acceptance tests to fetch non-public schemas; however, none of our tests actually have non-public schemas.

The final step for supporting non-public schemas at this point is validating everything is actually working through the acceptance tests, updating the documentation, and removing the gate.

Motivation

#94

Testing

Acceptance tests

Copy link
Collaborator Author

@bplunkett-stripe bplunkett-stripe Feb 1, 2024

Choose a reason for hiding this comment

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

We technically don't need to build a excludeSchemasFilter if there is an includeSchemasFilter, however I think the flow of control here is a bit simpler.

Everything will also still work if we got rid of the intersection check with this method! I.e., if you include and exclude a schema, then it makes sense that it would be excluded. (true && false) -> false

Copy link
Collaborator Author

@bplunkett-stripe bplunkett-stripe Feb 1, 2024

Choose a reason for hiding this comment

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

This is the core of the PR. Without this, acceptance tests would fail because they would try to diff out the metadata table.

for _, filter := range tc.filters {
filters = append(filters, newFakeNameFilter(t, filter).filter)
}
assert.Equal(t, tc.expectedOut, orNameFilter(filters...)(tc.input))

Choose a reason for hiding this comment

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

shouldn't this be "and" now? 😕

@bplunkett-stripe bplunkett-stripe merged commit e10c39e into main Feb 1, 2024
@bplunkett-stripe bplunkett-stripe deleted the bplunkett/add-exclude-schemas branch February 1, 2024 19:07
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.

3 participants