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

mysql: Disable mysql_integration_test until it works in more environments. #6250

Merged

Conversation

jmarantz
Copy link
Contributor

Description: This integration test consistently fails for some on macos, and for others on linux. It needs to be deflaked. In the meantime, commenting it out. See #6162
Risk Level: low
Testing: test/extensions/filters/network/mysql_proxy/...
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Joshua Marantz <jmarantz@google.com>
@jmarantz jmarantz requested a review from mattklein123 as a code owner March 11, 2019 17:27
@dnoe
Copy link
Contributor

dnoe commented Mar 11, 2019

Does it make more sense to disable the specific tests that have proven flakey by renaming those tests with disabled prefix? Doing it this way means the test .cc file isn't getting built, which could hide something. And unless all tests in that file are flakey then it would be better to disable only the ones that are.

@jmarantz
Copy link
Contributor Author

The advantage of what you suggest is that it might just be one test method, and we might find value in the other ones. The other advantage is that it's possible the original author can't repro the problem and there's something environmental about it.

But ideally, the test owner should ideally iterate on the exact problem. @venilnoronha is that you?

@jmarantz
Copy link
Contributor Author

I commented out the test that failed, and this integration test suite simply crashed on the next test, so I think this needs a deeper dive from the owner.

@jmarantz jmarantz assigned jmarantz and venilnoronha and unassigned jmarantz Mar 11, 2019
@jmarantz jmarantz changed the title mysql: Disable mysql_integration_test until it is deflaked. mysql: Disable mysql_integration_test until it works in more environments. Mar 11, 2019
@jmarantz
Copy link
Contributor Author

I changed the PR title as it does not appear to be a flake; it always fails.

@venilnoronha
Copy link
Member

@jmarantz I'm not the original author of the test, but I'll debug it.

@mattklein123 mattklein123 merged commit eff73b4 into envoyproxy:master Mar 12, 2019
spenceral added a commit to spenceral/envoy that referenced this pull request Mar 20, 2019
* master: (59 commits)
  http fault: add response rate limit injection (envoyproxy#6267)
  xds: introduce initial_fetch_timeout option to limit initialization time (envoyproxy#6048)
  test: fix cpuset-threads tests (envoyproxy#6278)
  server: add an API for registering for notifications for server instance life… (envoyproxy#6254)
  remove remains of TestBase (envoyproxy#6286)
  dubbo_proxy: Implement the routing of Dubbo requests (envoyproxy#5973)
  Revert "stats: add new BoolIndicator stat type (envoyproxy#5813)" (envoyproxy#6280)
  runtime: codifying runtime guarded features (envoyproxy#6134)
  mysql_filter: fix integration test flakes (envoyproxy#6272)
  tls: update BoringSSL to debed9a4 (3683). (envoyproxy#6273)
  rewrite buffer implementation to eliminate evbuffer dependency (envoyproxy#5441)
  Remove the dependency from TimeSystem to libevent by using the Event::Scheduler abstraction as a delegate. (envoyproxy#6240)
  fuzz: fix use of literal in default initialization. (envoyproxy#6268)
  http: add HCM functionality required for rate limiting (envoyproxy#6242)
  Disable mysql_integration_test until it is deflaked. (envoyproxy#6250)
  test: use ipv6_only IPv6 addresses in custom cluster integration tests. (envoyproxy#6260)
  tracing: If parent span is propagated with empty string, it causes th… (envoyproxy#6263)
  upstream: fix oss-fuzz issue envoyproxy#11095. (envoyproxy#6220)
  Wire up panic mode subset to receive updates (envoyproxy#6221)
  docs: clarify xds docs with warming information (envoyproxy#6236)
  ...
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.

4 participants