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

make our unit tests less brittle and not compare against messages out of our control #20009

Merged

Conversation

brianjlai
Copy link
Contributor

@brianjlai brianjlai commented Dec 1, 2022

What

We had some tests in the connector builder server that asserted on very specific error messages. Something mightve changed upstream with additional quotes. This is breaking the build.

How

I'm not 100% what changed slightly upstream, but at the end of the day we really shouldn't be having tests that look for specific text and especially when that text is dependent on an external package (in this case airbyte-cdk). We should just test what is mostly in our control like status code and part of the error text that our server is emitted.


assert actual_exception.value.status_code == 400
assert actual_exception.value.detail == "Could not perform read with with error: Every message grouping should have at least one request and response"
assert "Could not perform read with with error" in actual_exception.value.detail
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could even do away with checking any error text at all since text is often changing and error code tends to be more important

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to not testing the error message

@brianjlai brianjlai marked this pull request as ready for review December 1, 2022 23:44
@brianjlai brianjlai temporarily deployed to more-secrets December 2, 2022 00:15 Inactive
@brianjlai brianjlai merged commit 6dee382 into master Dec 2, 2022
@brianjlai brianjlai deleted the brian/make_connector_builder_error_tests_less_brittle branch December 2, 2022 00:55
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.

None yet

3 participants