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

Solve CI flakiness for test/java/org/chromium/net/urlconnection:urlconnection_test #2007

Merged
merged 5 commits into from
Jan 25, 2022

Conversation

carloseltuerto
Copy link
Contributor

@carloseltuerto carloseltuerto commented Jan 19, 2022

Reduce the number of concurrent requests for CronetUrlRequestTest.testManyRequests to 1000.

This solves this crash:

[2022-01-23 17:01:31.159][9613895][critical][assert] [external/envoy/source/common/network/connection_impl.cc:604] assert failure: !connecting_.

This ran on a 2017 MacBookPro without errors:

bazel test --define=signal_trace=disabled --runs_per_test=1000 test/java/org/chromium/net:cronet_url_request_test

However, this PR does not solve the root cause, it only mitigates it. It should not be possible for this ASSERT to fail.

Description: EM has a 1024 concurrent request limit that is now respected by Cronet tests
Risk Level: N/A
Testing: Solves CI "android_tests / java_tests_mac" failure
Docs Changes: N/A
Release Notes: N/A
Signed-off-by: Charles Le Borgne cleborgne@google.com

Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
Signed-off-by: Charles Le Borgne <cleborgne@google.com>
CronetUrlRequestTest.testManyRequests to 1000

Signed-off-by: Charles Le Borgne <cleborgne@google.com>
@carloseltuerto carloseltuerto changed the title Update Mockito release to solve CI Kotlin breakage Solve CI flakiness for test/java/org/chromium/net/urlconnection:urlconnection_test Jan 24, 2022
@goaway
Copy link
Contributor

goaway commented Jan 24, 2022

Thanks @carloseltuerto! For what it's worth, the limit is fairly arbitrary and I'm not opposed to increasing it.

@goaway goaway merged commit af4d895 into envoyproxy:main Jan 25, 2022
jpsim added a commit to jpsim/envoy-mobile that referenced this pull request Feb 3, 2022
* origin/main: (21 commits)
  owners: adding charles (envoyproxy#2038)
  [OWNERS] Update Envoy Mobile owners (envoyproxy#2031)
  [deps] Update rules_android to a stable release URL (envoyproxy#2032)
  [json] Remove com_github_nlohmann_json override & patch (envoyproxy#2030)
  Enable the "received byte count" feature. (envoyproxy#2004)
  Run Kotlin macOS tests without EngFlow (envoyproxy#2018)
  final_intel: adding response flags (envoyproxy#2009)
  fix (envoyproxy#2025)
  stream intel: update kotlin public interface to align with swift (envoyproxy#2013)
  tls: update bundled root certificates (envoyproxy#2016)
  tooling: first pass at oncall tooling (envoyproxy#2014)
  test: Solve CI flakiness for test/java/org/chromium/net/urlconnection:urlconnection_test  (envoyproxy#2007)
  envoy: 519774f742 (envoyproxy#2015)
  Default timestamp to -1 for envoy_final_stream_intel (envoyproxy#2006)
  QuicTestServer (envoyproxy#1989)
  bump boringssl to Envoy's version (envoyproxy#1999)
  docs: addding release notes (envoyproxy#2001)
  release: cutting the 0.4.5 release (envoyproxy#2000)
  Revert "bazel: change bazelisk for M1 support (envoyproxy#1966)" (envoyproxy#1998)
  Decompress even when `no-transform` is specified (envoyproxy#1995)
  ...

Signed-off-by: JP Simard <jp@jpsim.com>
@carloseltuerto carloseltuerto deleted the cronvoy056 branch February 15, 2022 19:25
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.

3 participants