-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Integration tests flaky in AppVeyor #1429
Comments
This seems to have been happening on every PR. |
We need to either fix the test or stop making this part of the build; it looks really bad to have a Beta product whose builds are failing. /cc @omaray |
I'll look into this. |
@garrettjonesgoogle I think this test file should simply be deleted. The goal of this repo is to create good client libraries. This file is not testing the DNS library. It's testing the DNS emulator, which was written just to test the client. I think the emulator is a maintenance burden. It's an HTTP server, meaning it has to deal with synchronization. It also tries to behave like the DNS service. Arguably, it's more complicated than the code it's trying to test. It's much easier to test the library by making sure that method calls create the right RPC requests and that RPC responses gets translated to the right response objects. We can integration test by calling a couple methods on the real service to make sure we set up HTTP and Auth layers properly. I think this would lead to much better tests. |
I looked through more of the AppVeyor failures. It does look like about half are the particular failure mentioned by @lesv , but I also see other ones too. They occur in ITTranslateTest, ITLoggingTest, LocalPubSubHelperTest, ITBigQueryTest, and BlockingProcessStreamReaderTest. So, just disabling LocalDnsHelperTest won't help. @mziccard , any insights? It seems like there is probably a fair amount of work necessary to work through all of the failures happening.
|
It looks like the DNS test in particular is so problematic because we have a DNS emulator that lives within google-cloud-java. It doesn't seem like a good idea for us to be maintaining our own emulator for someone else's service. I'm in favor of nuking it. |
New information: the AppVeyor configuration is set to run integration tests on commits, but not PRs. No AppVeyor run that included integration tests has gotten past the Translate failure, so that needs to be fixed. I am switching AppVeyor to only run unit tests for now because the integration tests are not currently providing value. |
It looks like users are seeing the Translate failure outside of integration tests: #1405 |
At this point, AppVeyor isn't even running tests - the queue is very backlogged. |
We should get AppVeyor integration tests up and running again for GA. |
With this PR, this issue should be resolved. |
…datatransfer to v2.3.2 (#1429) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [com.google.cloud:google-cloud-bigquerydatatransfer](https://togithub.com/googleapis/java-bigquerydatatransfer) | `2.2.4` -> `2.3.2` | [![age](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-bigquerydatatransfer/2.3.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-bigquerydatatransfer/2.3.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-bigquerydatatransfer/2.3.2/compatibility-slim/2.2.4)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/maven/com.google.cloud:google-cloud-bigquerydatatransfer/2.3.2/confidence-slim/2.2.4)](https://docs.renovatebot.com/merge-confidence/) | --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. ⚠ **Warning**: custom changes will be lost. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/java-bigquerydatatransfer). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xMzUuMSIsInVwZGF0ZWRJblZlciI6IjMyLjEzNS4xIn0=-->
#1421 AppVeyor failed on DNS for this, succeeded for Travis, I'm going to approve this.
getTtl()
appears to be Null.The text was updated successfully, but these errors were encountered: