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

feat: adds ForeignTypeInfo test #2076

Merged
merged 7 commits into from
Nov 25, 2024

Conversation

chalmerlowe
Copy link
Collaborator

@chalmerlowe chalmerlowe commented Nov 22, 2024

Adds a test to the test sets for ForeignTypeInfo.

  • from_api_repr

Also removed class TableSchema as unneeded.

@chalmerlowe chalmerlowe requested review from a team as code owners November 22, 2024 13:14
@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Nov 22, 2024
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Nov 22, 2024
@chalmerlowe chalmerlowe reopened this Nov 22, 2024
@chalmerlowe chalmerlowe changed the base branch from main to pangea-v1alpha November 22, 2024 13:16
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: xl Pull request size is extra large. labels Nov 22, 2024
@chalmerlowe chalmerlowe changed the title Feat b358215039 add foreigntypeinfo test feat: adds ForeignTypeInfo test Nov 22, 2024
@@ -598,61 +598,6 @@ def to_api_repr(self) -> dict:
return answer


class TableSchema:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: with this much removed code that isnt relevant to the change, it would probably make sense to have that as a separate PR (for next time don't need to move it now)

klass = self._get_target_class()
result = klass.from_api_repr(resource)

assert result.to_api_repr() == expected.to_api_repr()
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar feedback to the other CL, though maybe though now I'm thinking maybe we need to test that from_api_repr has the same response, though since to_api_repr is tested I think this is actually ok.

Your doc comment does describe that the results we are comparing are displayed as a dict, maybe its just the test naming that is throwing it off for me (maybe test_convert_api_repr or something?). Either way do think its fine to leave as is.

@chalmerlowe chalmerlowe added the kokoro:run Add this label to force Kokoro to re-run the tests. label Nov 25, 2024
@chalmerlowe chalmerlowe merged commit 8162a2c into pangea-v1alpha Nov 25, 2024
19 of 20 checks passed
@chalmerlowe chalmerlowe deleted the feat-b358215039-add-foreigntypeinfo-test branch November 25, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. kokoro:run Add this label to force Kokoro to re-run the tests. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants