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

Add more YAML verification #5151

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

bengl
Copy link
Collaborator

@bengl bengl commented Jan 23, 2025

  • Validate YAML files using actionlint
  • Check that tests in packages/datadog-instrumentations/tests/ are actually run.
  • Check that tests in packages/datadog-plugin-*/tests/ are actually run.
  • Ensure all tests have the same triggers.

@bengl bengl requested review from a team as code owners January 23, 2025 19:29
env:
PLUGINS: passport-http
steps:
- uses: actions/checkout@v4
Copy link

@datadog-datadog-prod-us1 datadog-datadog-prod-us1 bot Jan 23, 2025

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

View in Datadog  Leave us feedback  Documentation


- name: actionlint
id: actionlint #optional, id required only when outputs are used in the workflow steps later
uses: raven-actions/actionlint@v2

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

View in Datadog  Leave us feedback  Documentation

Copy link
Collaborator

Choose a reason for hiding this comment

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

??

Copy link

github-actions bot commented Jan 23, 2025

Overall package size

Self size: 8.53 MB
Deduped: 94.89 MB
No deduping: 95.41 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.2.0 | 13.9 MB | 13.91 MB | | @datadog/pprof | 5.4.1 | 9.76 MB | 10.13 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.6.1 | 2.59 MB | 2.73 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 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.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 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

@pr-commenter
Copy link

pr-commenter bot commented Jan 23, 2025

Benchmarks

Benchmark execution time: 2025-01-24 04:53:06

Comparing candidate commit 1b56cf8 in PR branch bengl/more-yaml-verification with baseline commit 30efc06 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 905 metrics, 28 unstable metrics.

@bengl bengl force-pushed the bengl/more-yaml-verification branch 2 times, most recently from 9712db5 to 75b9f4b Compare January 23, 2025 20:09
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.45%. Comparing base (c28765a) to head (75b9f4b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #5151       +/-   ##
===========================================
- Coverage   95.02%   62.45%   -32.57%     
===========================================
  Files         108      336      +228     
  Lines        3475    15557    +12082     
===========================================
+ Hits         3302     9716     +6414     
- Misses        173     5841     +5668     

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

@bengl bengl requested a review from a team as a code owner January 23, 2025 20:20
env:
PLUGINS: multer
steps:
- uses: actions/checkout@v4

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

View in Datadog  Leave us feedback  Documentation

bengl added 5 commits January 23, 2025 16:45
* Validate YAML files using actionlint
* Check that tests in packages/datadog-instrumentations/tests/ are
  actually run.
* Ensure all tests have the same triggers.
It was testing very specific output, when really it should have just
looked for its own output.
@bengl bengl force-pushed the bengl/more-yaml-verification branch from 83cc0ba to 52f0f5a Compare January 23, 2025 21:45
@@ -15,7 +15,7 @@ withVersions('multer', 'multer', version => {
})

before((done) => {
const express = require('../../../versions/express').get()
const express = require('express')
Copy link
Member

Choose a reason for hiding this comment

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

why this change ? shouldn't multer decide what version it's supposed to use from the external.json file thingy ?

Copy link
Member

Choose a reason for hiding this comment

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

wouldn't that just use the express we have in our package.json ?

@@ -30,6 +45,9 @@ function checkYaml (yamlPath) {
if (!job.env || !job.env.PLUGINS) continue

const pluginName = job.env.PLUGINS
if (!yamlPath.includes('appsec')) {
Copy link
Member

Choose a reason for hiding this comment

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

why not check appsec ones too ?

Copy link
Member

Choose a reason for hiding this comment

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

btw is this script even running in the CI ?

@simon-id
Copy link
Member

few comments otherwise LGTM

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