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

[ci-visibility] Add get known tests request #4015

Merged
merged 4 commits into from
Feb 5, 2024

Conversation

juan-fernandez
Copy link
Collaborator

@juan-fernandez juan-fernandez commented Feb 1, 2024

What does this PR do?

Add get known tests logic.

It's the first piece of the "Early flake detection" feature:

  • we detect what tests are new in your feature branch
  • we run them multiple times to check if they're flaky to stop you from merging flaky tests to the default branch

This PR just adds the necessary logic to request the known tests for a given test service (so the test framework can be aware of what tests are new).

Motivation

The early flake detection changes are considerable, so I'm splitting the changes in different PRs so it's easier to review.

This PR:

  • Adds getKnownTests request.
  • Adds getKnownTests to the ci visibility exporter, so plugins can request known tests.
  • Adds corresponding tests.

This PR does not include usage of the known tests by any test framework.

Plugin Checklist

  • Unit tests.

Security

Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

Copy link

github-actions bot commented Feb 1, 2024

Overall package size

Self size: 5.97 MB
Deduped: 61.67 MB
No deduping: 62.42 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.6.4 16.43 MB 16.44 MB
@datadog/native-appsec 7.0.0 14.51 MB 14.52 MB
@datadog/pprof 5.0.0 9.59 MB 10.44 MB
protobufjs 7.2.5 2.77 MB 6.56 MB
@datadog/native-iast-rewriter 2.2.2 2.29 MB 2.37 MB
@opentelemetry/core 1.14.0 872.87 kB 1.47 MB
@datadog/native-metrics 2.0.0 898.77 kB 1.3 MB
@opentelemetry/api 1.4.1 780.32 kB 780.32 kB
import-in-the-middle 1.7.3 67.62 kB 731.01 kB
pprof-format 2.0.7 588.12 kB 588.12 kB
msgpack-lite 0.1.26 201.16 kB 281.59 kB
opentracing 0.14.7 194.81 kB 194.81 kB
semver 7.5.4 93.4 kB 123.8 kB
@datadog/sketches-js 2.1.0 109.9 kB 109.9 kB
lodash.sortby 4.7.0 75.76 kB 75.76 kB
lru-cache 7.14.0 74.95 kB 74.95 kB
ipaddr.js 2.1.0 60.23 kB 60.23 kB
ignore 5.2.4 51.22 kB 51.22 kB
int64-buffer 0.1.10 49.18 kB 49.18 kB
shell-quote 1.8.1 44.96 kB 44.96 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
tlhunter-sorted-set 0.1.0 24.94 kB 24.94 kB
limiter 1.1.5 23.17 kB 23.17 kB
dc-polyfill 0.1.2 22.77 kB 22.77 kB
retry 0.13.1 18.85 kB 18.85 kB
node-abort-controller 3.1.1 16.89 kB 16.89 kB
jest-docblock 29.7.0 8.99 kB 12.76 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
path-to-regexp 0.1.7 6.78 kB 6.78 kB
koalas 1.0.2 6.47 kB 6.47 kB
methods 1.1.2 5.29 kB 5.29 kB
module-details-from-path 1.0.3 4.47 kB 4.47 kB

🤖 This report was automatically generated by heaviest-objects-in-the-universe

Copy link

codecov bot commented Feb 1, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (6bfad8f) 85.28% compared to head (f06b528) 85.15%.
Report is 5 commits behind head on master.

Files Patch % Lines
...isibility/early-flake-detection/get-known-tests.js 82.60% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4015      +/-   ##
==========================================
- Coverage   85.28%   85.15%   -0.13%     
==========================================
  Files         242      243       +1     
  Lines       10452    10490      +38     
  Branches       33       33              
==========================================
+ Hits         8914     8933      +19     
- Misses       1538     1557      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pr-commenter
Copy link

pr-commenter bot commented Feb 1, 2024

Benchmarks

Benchmark execution time: 2024-02-01 16:26:32

Comparing candidate commit f06b528 in PR branch juan-fernandez/add-get-known-tests with baseline commit 6bfad8f in branch master.

Found 2 performance improvements and 1 performance regressions! Performance is the same for 257 metrics, 6 unstable metrics.

scenario:plugin-graphql-with-depth-off-18

  • 🟩 max_rss_usage [-130.934MB; -126.350MB] or [-13.540%; -13.066%]

scenario:plugin-graphql-with-depth-on-max-18

  • 🟩 max_rss_usage [-118.424MB; -108.400MB] or [-12.497%; -11.439%]

scenario:scope-manager-async_local_storage-18

  • 🟥 cpu_user_time [+5.313ms; +12.440ms] or [+5.762%; +13.493%]

@juan-fernandez juan-fernandez changed the title [ci-visibility] Add get known tests [ci-visibility] Add get known tests request Feb 1, 2024
@juan-fernandez juan-fernandez force-pushed the juan-fernandez/add-get-known-tests branch from cbabf92 to ab8b91d Compare February 1, 2024 13:52
@juan-fernandez juan-fernandez force-pushed the juan-fernandez/add-get-known-tests branch from ab8b91d to 75f8356 Compare February 1, 2024 13:53
@juan-fernandez juan-fernandez marked this pull request as ready for review February 1, 2024 13:58
@juan-fernandez juan-fernandez requested review from a team as code owners February 1, 2024 13:58
anmarchenko
anmarchenko previously approved these changes Feb 1, 2024
@@ -115,6 +115,18 @@ module.exports = class CiPlugin extends Plugin {
})
this.telemetry.count(TELEMETRY_ITR_SKIPPED, { testLevel: 'suite' }, skippedSuites.length)
})

this.addSub(`ci:${this.constructor.id}:known-tests`, ({ onDone }) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, but it's a bit difficult to know why this is needed without any publisher. Generally speaking it's a pattern that should be avoided, so just pointing it out to make sure other options were evaluated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good point. The changes are considerable so I split the PRs, leaving this is as a noop, which isn't great. The usage will become clear very soon though, as I have the changes ready :)

@juan-fernandez juan-fernandez merged commit 2a9b541 into master Feb 5, 2024
110 of 111 checks passed
@juan-fernandez juan-fernandez deleted the juan-fernandez/add-get-known-tests branch February 5, 2024 16:33
@tlhunter tlhunter mentioned this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants