memory: use EXPECT_LE to measure memory consumption to allow for differing versions of STL etc#7208
Conversation
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
cmluciano
left a comment
There was a problem hiding this comment.
The CI errors are related to failed pulls of nodejs deps.
bazel test //test/common/stats:symbol_table_impl_test & bazel test //test/integration:stats_integration_test are passing locally for me with clang-8 & bazel 0.27.0rc3
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
I triggered a build on Bazel CI for your PR and it's green again! 😃 Here are the results: https://buildkite.com/bazel/envoy/builds/414 (Ignore the last failing step regarding "Try Update Last Green Commit" - that's because I manually triggered it and your commit isn't merged yet.) |
| // 2019/06/06 7208 49419 make memory targets approximate | ||
|
|
||
| EXPECT_EQ(m_per_cluster, 49393); | ||
| EXPECT_LE(m_per_cluster, 49449); |
There was a problem hiding this comment.
We will miss some regression on this since PRs saving memory likely won't update the value here. I don't really have idea to make this better while make it robust. Shall we do some exact match when it is running in our CI image only?
There was a problem hiding this comment.
Sure but I think that can be a follow-up. Maybe we could have a test-helper method that took the exact number we scrape from CI, but did an inexact comparison when not running in CI.
|
hmm, the magic number fails on macOS too @jmarantz |
Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
/retest |
|
🔨 rebuilding |
Description: It appears that CI (Bazel & Envoy projects) has diverged in its memory behavior from our development environment behavior, so we need to rely on EXPECT_LE rather than EXPECT_EQ for tracking memory usage. Otherwise we inherit a boatload of maintenance and determinism hassle trying to keep CI and dev builds working.
Risk Level: low
Testing: //test/... finished (default build). --config libc++ running now in parallel.
Docs Changes: n/a
Release Notes: n/a
Fixes: #7196