-
Notifications
You must be signed in to change notification settings - Fork 293
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
Request Test Management tests list #8345
base: master
Are you sure you want to change the base?
Request Test Management tests list #8345
Conversation
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 54 metrics, 9 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.042 s) : 0, 1042288
Total [baseline] (10.445 s) : 0, 10444760
Agent [candidate] (1.042 s) : 0, 1042485
Total [candidate] (10.452 s) : 0, 10452233
section appsec
Agent [baseline] (1.184 s) : 0, 1183692
Total [baseline] (10.671 s) : 0, 10671163
Agent [candidate] (1.19 s) : 0, 1190238
Total [candidate] (10.768 s) : 0, 10767676
section iast
Agent [baseline] (1.184 s) : 0, 1184210
Total [baseline] (10.948 s) : 0, 10947947
Agent [candidate] (1.175 s) : 0, 1175438
Total [candidate] (10.921 s) : 0, 10921086
section profiling
Agent [baseline] (1.261 s) : 0, 1261016
Total [baseline] (10.815 s) : 0, 10815104
Agent [candidate] (1.259 s) : 0, 1259055
Total [candidate] (10.782 s) : 0, 10782471
gantt
title petclinic - break down per module: candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (715.127 ms) : 0, 715127
BytebuddyAgent [candidate] (715.261 ms) : 0, 715261
GlobalTracer [baseline] (242.615 ms) : 0, 242615
GlobalTracer [candidate] (243.179 ms) : 0, 243179
AppSec [baseline] (55.158 ms) : 0, 55158
AppSec [candidate] (55.168 ms) : 0, 55168
Remote Config [baseline] (711.693 µs) : 0, 712
Remote Config [candidate] (724.291 µs) : 0, 724
Telemetry [baseline] (13.525 ms) : 0, 13525
Telemetry [candidate] (13.027 ms) : 0, 13027
section appsec
BytebuddyAgent [baseline] (732.584 ms) : 0, 732584
BytebuddyAgent [candidate] (736.104 ms) : 0, 736104
GlobalTracer [baseline] (239.783 ms) : 0, 239783
GlobalTracer [candidate] (241.655 ms) : 0, 241655
AppSec [baseline] (175.647 ms) : 0, 175647
AppSec [candidate] (177.358 ms) : 0, 177358
Remote Config [baseline] (651.089 µs) : 0, 651
Remote Config [candidate] (666.813 µs) : 0, 667
Telemetry [baseline] (8.973 ms) : 0, 8973
Telemetry [candidate] (8.32 ms) : 0, 8320
IAST [baseline] (21.603 ms) : 0, 21603
IAST [candidate] (21.744 ms) : 0, 21744
section iast
BytebuddyAgent [baseline] (842.061 ms) : 0, 842061
BytebuddyAgent [candidate] (834.789 ms) : 0, 834789
GlobalTracer [baseline] (235.842 ms) : 0, 235842
GlobalTracer [candidate] (235.145 ms) : 0, 235145
AppSec [baseline] (58.0 ms) : 0, 58000
AppSec [candidate] (57.896 ms) : 0, 57896
Remote Config [baseline] (629.164 µs) : 0, 629
Remote Config [candidate] (643.88 µs) : 0, 644
Telemetry [baseline] (8.878 ms) : 0, 8878
Telemetry [candidate] (8.769 ms) : 0, 8769
IAST [baseline] (23.265 ms) : 0, 23265
IAST [candidate] (22.951 ms) : 0, 22951
section profiling
BytebuddyAgent [baseline] (705.781 ms) : 0, 705781
BytebuddyAgent [candidate] (704.49 ms) : 0, 704490
GlobalTracer [baseline] (352.594 ms) : 0, 352594
GlobalTracer [candidate] (352.997 ms) : 0, 352997
AppSec [baseline] (55.138 ms) : 0, 55138
AppSec [candidate] (54.175 ms) : 0, 54175
Remote Config [baseline] (706.913 µs) : 0, 707
Remote Config [candidate] (699.0 µs) : 0, 699
Telemetry [baseline] (8.962 ms) : 0, 8962
Telemetry [candidate] (8.924 ms) : 0, 8924
ProfilingAgent [baseline] (95.567 ms) : 0, 95567
ProfilingAgent [candidate] (95.583 ms) : 0, 95583
Profiling [baseline] (95.591 ms) : 0, 95591
Profiling [candidate] (95.607 ms) : 0, 95607
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.044 s) : 0, 1044486
Total [baseline] (8.668 s) : 0, 8667534
Agent [candidate] (1.041 s) : 0, 1040949
Total [candidate] (8.618 s) : 0, 8617752
section iast
Agent [baseline] (1.171 s) : 0, 1171497
Total [baseline] (9.175 s) : 0, 9174730
Agent [candidate] (1.169 s) : 0, 1169162
Total [candidate] (9.214 s) : 0, 9214297
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.175 s) : 0, 1174512
Total [baseline] (9.243 s) : 0, 9242597
Agent [candidate] (1.171 s) : 0, 1171044
Total [candidate] (9.18 s) : 0, 9179733
section iast_TELEMETRY_OFF
Agent [baseline] (1.167 s) : 0, 1166988
Total [baseline] (9.209 s) : 0, 9208716
Agent [candidate] (1.166 s) : 0, 1165846
Total [candidate] (9.205 s) : 0, 9205094
gantt
title insecure-bank - break down per module: candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (716.257 ms) : 0, 716257
BytebuddyAgent [candidate] (714.148 ms) : 0, 714148
GlobalTracer [baseline] (244.167 ms) : 0, 244167
GlobalTracer [candidate] (242.778 ms) : 0, 242778
AppSec [baseline] (55.667 ms) : 0, 55667
AppSec [candidate] (55.959 ms) : 0, 55959
Remote Config [baseline] (727.264 µs) : 0, 727
Remote Config [candidate] (728.17 µs) : 0, 728
Telemetry [baseline] (12.503 ms) : 0, 12503
Telemetry [candidate] (12.211 ms) : 0, 12211
section iast
BytebuddyAgent [baseline] (834.177 ms) : 0, 834177
BytebuddyAgent [candidate] (832.575 ms) : 0, 832575
GlobalTracer [baseline] (233.414 ms) : 0, 233414
GlobalTracer [candidate] (233.227 ms) : 0, 233227
IAST [baseline] (22.75 ms) : 0, 22750
IAST [candidate] (22.527 ms) : 0, 22527
AppSec [baseline] (56.709 ms) : 0, 56709
AppSec [candidate] (56.26 ms) : 0, 56260
Remote Config [baseline] (637.778 µs) : 0, 638
Remote Config [candidate] (618.893 µs) : 0, 619
Telemetry [baseline] (8.633 ms) : 0, 8633
Telemetry [candidate] (8.639 ms) : 0, 8639
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (834.185 ms) : 0, 834185
BytebuddyAgent [candidate] (833.642 ms) : 0, 833642
GlobalTracer [baseline] (234.283 ms) : 0, 234283
GlobalTracer [candidate] (233.546 ms) : 0, 233546
IAST [baseline] (23.166 ms) : 0, 23166
IAST [candidate] (24.814 ms) : 0, 24814
AppSec [baseline] (58.146 ms) : 0, 58146
AppSec [candidate] (54.489 ms) : 0, 54489
Remote Config [baseline] (648.4 µs) : 0, 648
Remote Config [candidate] (628.847 µs) : 0, 629
Telemetry [baseline] (8.849 ms) : 0, 8849
Telemetry [candidate] (8.669 ms) : 0, 8669
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (830.419 ms) : 0, 830419
BytebuddyAgent [candidate] (829.443 ms) : 0, 829443
GlobalTracer [baseline] (232.962 ms) : 0, 232962
GlobalTracer [candidate] (233.245 ms) : 0, 233245
IAST [baseline] (26.148 ms) : 0, 26148
IAST [candidate] (27.567 ms) : 0, 27567
AppSec [baseline] (52.992 ms) : 0, 52992
AppSec [candidate] (51.214 ms) : 0, 51214
Remote Config [baseline] (626.545 µs) : 0, 627
Remote Config [candidate] (621.973 µs) : 0, 622
Telemetry [baseline] (8.607 ms) : 0, 8607
Telemetry [candidate] (8.601 ms) : 0, 8601
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 16 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section baseline
no_agent (380.723 µs) : 361, 401
. : milestone, 381,
iast (510.864 µs) : 489, 533
. : milestone, 511,
iast_FULL (744.274 µs) : 722, 766
. : milestone, 744,
iast_GLOBAL (558.097 µs) : 536, 580
. : milestone, 558,
iast_HARDCODED_SECRET_DISABLED (511.448 µs) : 490, 533
. : milestone, 511,
iast_INACTIVE (461.113 µs) : 440, 482
. : milestone, 461,
iast_TELEMETRY_OFF (502.677 µs) : 479, 527
. : milestone, 503,
tracing (464.368 µs) : 442, 487
. : milestone, 464,
section candidate
no_agent (381.104 µs) : 361, 401
. : milestone, 381,
iast (512.372 µs) : 491, 534
. : milestone, 512,
iast_FULL (746.289 µs) : 725, 768
. : milestone, 746,
iast_GLOBAL (551.729 µs) : 530, 573
. : milestone, 552,
iast_HARDCODED_SECRET_DISABLED (510.646 µs) : 489, 532
. : milestone, 511,
iast_INACTIVE (460.142 µs) : 439, 481
. : milestone, 460,
iast_TELEMETRY_OFF (498.126 µs) : 476, 521
. : milestone, 498,
tracing (459.499 µs) : 438, 481
. : milestone, 459,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section baseline
no_agent (1.358 ms) : 1338, 1377
. : milestone, 1358,
appsec (1.765 ms) : 1742, 1789
. : milestone, 1765,
appsec_no_iast (1.748 ms) : 1724, 1773
. : milestone, 1748,
iast (1.521 ms) : 1496, 1545
. : milestone, 1521,
profiling (1.6 ms) : 1575, 1625
. : milestone, 1600,
tracing (1.5 ms) : 1475, 1525
. : milestone, 1500,
section candidate
no_agent (1.358 ms) : 1338, 1377
. : milestone, 1358,
appsec (1.748 ms) : 1724, 1771
. : milestone, 1748,
appsec_no_iast (1.776 ms) : 1752, 1800
. : milestone, 1776,
iast (1.529 ms) : 1505, 1552
. : milestone, 1529,
profiling (1.574 ms) : 1549, 1598
. : milestone, 1574,
tracing (1.482 ms) : 1457, 1507
. : milestone, 1482,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section baseline
no_agent (15.518 s) : 15518000, 15518000
. : milestone, 15518000,
appsec (15.165 s) : 15165000, 15165000
. : milestone, 15165000,
iast (18.738 s) : 18738000, 18738000
. : milestone, 18738000,
iast_GLOBAL (18.064 s) : 18064000, 18064000
. : milestone, 18064000,
profiling (15.676 s) : 15676000, 15676000
. : milestone, 15676000,
tracing (14.967 s) : 14967000, 14967000
. : milestone, 14967000,
section candidate
no_agent (14.919 s) : 14919000, 14919000
. : milestone, 14919000,
appsec (14.908 s) : 14908000, 14908000
. : milestone, 14908000,
iast (18.931 s) : 18931000, 18931000
. : milestone, 18931000,
iast_GLOBAL (18.28 s) : 18280000, 18280000
. : milestone, 18280000,
profiling (15.58 s) : 15580000, 15580000
. : milestone, 15580000,
tracing (14.858 s) : 14858000, 14858000
. : milestone, 14858000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.47.0-SNAPSHOT~428865d243, baseline=1.47.0-SNAPSHOT~26311f724d
dateFormat X
axisFormat %s
section baseline
no_agent (1.466 ms) : 1455, 1478
. : milestone, 1466,
appsec (2.347 ms) : 2303, 2391
. : milestone, 2347,
iast (2.115 ms) : 2059, 2171
. : milestone, 2115,
iast_GLOBAL (2.158 ms) : 2102, 2214
. : milestone, 2158,
profiling (2.458 ms) : 2269, 2647
. : milestone, 2458,
tracing (1.951 ms) : 1908, 1994
. : milestone, 1951,
section candidate
no_agent (1.469 ms) : 1458, 1480
. : milestone, 1469,
appsec (2.349 ms) : 2305, 2393
. : milestone, 2349,
iast (2.11 ms) : 2055, 2165
. : milestone, 2110,
iast_GLOBAL (2.151 ms) : 2096, 2207
. : milestone, 2151,
profiling (1.956 ms) : 1912, 2000
. : milestone, 1956,
tracing (1.944 ms) : 1902, 1987
. : milestone, 1944,
|
@@ -50,5 +56,8 @@ Map<String, Collection<TestIdentifier>> getFlakyTestsByModule(TracerEnvironment | |||
Map<String, Collection<TestIdentifier>> getKnownTestsByModule(TracerEnvironment tracerEnvironment) | |||
throws IOException; | |||
|
|||
Map<String, Map<String, Collection<TestIdentifier>>> getTestManagementTestsByModule( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Map<String, Map<String, Collection<TestIdentifier>>>
might be difficult to comprehend when we revisit this code in a few months :)
Let's see if we can apply stronger typing, e.g. something along the lines of Map<String, Map<TestManagementStatus, Collection<TestIdentifier>>>
.
Where TestManagementStatus
is an enum with possible values quarantined
, disabled
, attemptToFix
(TestManagementStatus
might not be the best name for it, we can consider alternatives too, like TestManagementStrategy
or TestManagementBehavior
).
My main point is what we expect here is a set of values that are known to the tracer, if we cement this with some compile-time safety, the code will be easier to reason about.
TracerEnvironment tracerEnvironment) throws IOException { | ||
OkHttpUtils.CustomListener telemetryListener = | ||
new TelemetryListener.Builder(metricCollector) | ||
.requestCount(CiVisibilityCountMetric.TEST_MANAGEMENT_TESTS_DETECTION_REQUEST) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "detection" made sense in "Impacted Tests Detection" as that was the name of the feature.
Here, I think, TEST_MANAGEMENT_TESTS_REQUEST
is enough
@Nullable private final Collection<TestIdentifier> flakyTests; | ||
@Nullable private final Collection<TestIdentifier> knownTests; | ||
@Nonnull private final Collection<TestIdentifier> quarantinedTests; | ||
@Nonnull private final Collection<TestIdentifier> disabledTests; | ||
@Nonnull private final Collection<TestIdentifier> attemptToFixTests; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats' a lot of collections, I'm concerned about the possible memory footprint.
I think it'd be more efficient (and concise and easier to comprehend too) to have a single Map<TestIdentifer, TestSettings>
where the value would contain all the possible settings for a given test (is flaky, is known, is quarantined, is disabled, is attempted to fix).
Could even be a map of <TestIdentifier, Integer>
where integer is a bit mask (a bit low level, but memory-efficient, and the complexity is tolerable if we encapsulate it in this class and expose a higher-level API).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
skippableTests
should be separate though, as there the identifier includes test parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which makes me think of another issue: in some places where TestIdentifier
is used, it includes parameters, while in others the parameters are omitted (because the backend doesn't give us that data).
It's not good that we have to guess whether a particular lookup should be done with parameters or without them, and the more logic we have using TestIdentifier
, the more convoluted things become.
Perhaps it is the right time to introduce something like TestFQN
(meaning "fully-qualified name"; as usual, naming things is the hard part) - something that identifies a test not taking its parameters into account - to make it clear what is expected where.
And then we could have something like TestIdentifer#toFQN
to convert one thing into the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should flakyTests
also be separate? I see that they can include test parameters as well in the request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, but it seems like they are never used with the parameters (the comparison is always done with TestIdentifier#withoutParameters
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't think the backend returns parameters for flaky tests. The test case is not accurate
Map<String, Collection<TestIdentifier>> quarantinedTestsByModule = | ||
testManagementTestsByModule != null | ||
? testManagementTestsByModule.get("quarantined") | ||
: Collections.emptyMap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If getTestManagementTestsByModule
returned an empty map in case of failure, we could simplify this bit (and the others here) to
testManagementTestsByModule.getOrDefault("quarantined", Collections.emptyMap());
"test-c": { | ||
"properties": { | ||
"quarantined": true, | ||
"disabled": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's maybe have one more example here where the flags are omitted rather than set to false
, as this is what the backend will probably do.
What Does This Do
Motivation
Second step in the implementation of Flaky Test Management.
Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: SDTEST-1530