test: make _AsyncJob tests mock at a lower layer
#340
Conversation
This is intented to make the `_AsyncJob` tests more robust to changes in retry behavior. It also more explicitly tests the retry behavior by observing API calls rather than calls to certain methods.
google-cla
bot
added
the
cla: yes
| # policy passed to it, so we don't throw a non-retriable | ||
| # exception here. See: | ||
| # https://github.com/googleapis/python-bigquery/issues/24 | ||
| _make_retriable_exception(), |
| @@ -588,7 +587,7 @@ def _check_resource_config(cls, resource): | |||
|
|
|||
| def to_api_repr(self): | |||
| """Generate a resource for the job.""" | |||
| raise NotImplementedError("Abstract") | |||
| return copy.deepcopy(self._properties) | |||
|
|
|||
| _build_resource = to_api_repr # backward-compatibility alias | |||
There was a problem hiding this comment.
This all LGTM, but I'm curious what this backwards compatibility is for here. It's not clear to me why we were specifically testing to make sure this would raise NotImplementedError before.
There was a problem hiding this comment.
It's not clear to me why we were specifically testing to make sure this would raise NotImplementedError before.
I think that was solely to make sure the coverage tests pass. We're still doing a bit of funny business in the *Job classes, in that most (all?) subclasses omit the statistics property from their to_api_repr.
I'm curious what this backwards compatibility is for here.
I don't remember, actually. I see a few hits for _build_resource in search. https://github.com/googleapis/python-bigquery/search?p=1&q=_build_resource I think _build_resource is usually the name we use for "update" methods that need to take a field mask and only populate certain fields. Jobs don't support update, so not sure why we ever had one.
This enables checking the job status without making an API call. It also fixes an inconsistency in `QueryJob`, where a job can be reported as "done" without having the results of a `getQueryResults` API call. Follow-up to #340
This is intented to make the
_AsyncJobtests more robustto changes in retry behavior. It also more explicitly tests
the retry behavior by observing API calls rather than calls
to certain methods.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #339 馃