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

switch to tap for ci/core/tracing/profiling tests #2889

Merged
merged 7 commits into from
Mar 16, 2023
Merged

Conversation

rochdev
Copy link
Member

@rochdev rochdev commented Mar 13, 2023

What does this PR do?

Update some tests to run in isolated workers using tap. There is also no global setup anymore, so individual tests files can be run directly with node somefile.spec.js.

Motivation

We need a way to isolate tests so that we can write integration tests without complex logic to cleanup after the suite, which is also not always possible.

Additional Notes

This is exactly the same code as #2538 but without AppSec as there are new test failures that I wasn't able to fix. I also wasn't able to merge properly as there were too many changes on the main branch, so I decided to open a new PR, but again it's the same code as #2538 so any review that was done there applies here as well.

@rochdev rochdev requested a review from a team as a code owner March 13, 2023 19:29
@github-actions
Copy link

github-actions bot commented Mar 13, 2023

Overall package size

Self size: 3.88 MB
Deduped: 57.55 MB
No deduping: 57.59 MB

Dependency sizes

name version self size total size
@datadog/native-iast-taint-tracking 1.1.1 13.38 MB 13.39 MB
@datadog/native-appsec 2.0.0 12.33 MB 12.34 MB
@datadog/pprof 2.0.0 10.47 MB 11.35 MB
@datadog/native-metrics 1.5.0 7.1 MB 7.11 MB
protobufjs 7.1.2 2.76 MB 6.55 MB
@datadog/native-iast-rewriter 2.0.1 2.09 MB 2.1 MB
opentracing 0.14.7 194.81 kB 194.81 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
semver 5.7.1 61.58 kB 61.58 kB
ipaddr.js 2.0.1 59.52 kB 59.52 kB
ignore 5.2.0 48.87 kB 48.87 kB
import-in-the-middle 1.3.4 32.7 kB 37.17 kB
istanbul-lib-coverage 3.2.0 29.34 kB 29.34 kB
retry 0.10.1 27.44 kB 27.44 kB
lodash.uniq 4.5.0 25.01 kB 25.01 kB
limiter 1.1.5 23.17 kB 23.17 kB
lodash.kebabcase 4.1.1 17.75 kB 17.75 kB
lodash.pick 4.4.0 16.33 kB 16.33 kB
node-abort-controller 3.0.1 14.33 kB 14.33 kB
crypto-randomuuid 1.0.0 11.18 kB 11.18 kB
diagnostics_channel 1.1.0 7.07 kB 7.07 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

@codecov
Copy link

codecov bot commented Mar 13, 2023

Codecov Report

Merging #2889 (77fc89f) into master (7b22c3b) will decrease coverage by 5.33%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2889      +/-   ##
==========================================
- Coverage   89.30%   83.98%   -5.33%     
==========================================
  Files         302       17     -285     
  Lines       11091      668   -10423     
  Branches       33       33              
==========================================
- Hits         9905      561    -9344     
+ Misses       1186      107    -1079     

see 285 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pr-commenter
Copy link

pr-commenter bot commented Mar 13, 2023

Benchmarks

Comparing candidate commit 77fc89f in PR branch mocha-to-tap with baseline commit 75c8a3a in branch master.

Found 0 performance improvements and 2 performance regressions! Performance is the same for 664 metrics, 42 unstable metrics.

scenario:exporting-pipeline-0.5-18

  • 🟥 cpu_user_time [+23.863ms; +28.763ms] or [+8.079%; +9.738%]
  • 🟥 execution_time [+26.162ms; +27.308ms] or [+8.124%; +8.480%]

Qard
Qard previously approved these changes Mar 13, 2023
@rochdev
Copy link
Member Author

rochdev commented Mar 13, 2023

@tlhunter For the comment in #2538 (comment): datadog-core has its own setup file, so that's the file that requires ../setup/tap.

@bengl For the comment in #2538 (comment) it was fixed a long time ago.

@juan-fernandez Please confirm if moving to tap works for you, if not I can just remove CI from the PR, although this should enable you to run tests in isolation without having to rely on the integration tests.

@simon-id I removed AppSec since it was failing, but I still moved it to its own jobs, so you can decide to switch to tap as well after this PR is merged or keep Mocha for your team.

juan-fernandez
juan-fernandez previously approved these changes Mar 14, 2023
Copy link
Collaborator

@juan-fernandez juan-fernandez left a comment

Choose a reason for hiding this comment

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

LGTM from CI Vis' perspective. I only worry about a test we're skipping. Just to confirm: this PR is a noop for plugins test, right?

@rochdev
Copy link
Member Author

rochdev commented Mar 14, 2023

Just to confirm: this PR is a noop for plugins test, right?

Correct, at least for now.

@rochdev rochdev dismissed stale reviews from juan-fernandez and Qard via 20ba63d March 14, 2023 15:58
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.

3 participants