ASCII-808: Linter ensures fxutil.Run is tested using TestRun #21461
Conversation
Bloop Bleep... Dogbot HereRegression Detector ResultsRun ID: 2dfe1955-0e05-44fe-9f79-9b959287416a Performance changes are noted in the perf column of each table:
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | confidence |
|---|---|---|---|---|---|
| ✅ | otel_to_otel_logs | ingress throughput | +1.99 | [+1.28, +2.71] | 100.00% |
Experiments with missing or malformed data
- idle
Usually, this warning means that there is no usable optimization goal data for that experiment, which could be a result of misconfiguration.
Fine details of change detection per experiment
| perf | experiment | goal | Δ mean % | Δ mean % CI | confidence |
|---|---|---|---|---|---|
| ✅ | otel_to_otel_logs | ingress throughput | +1.99 | [+1.28, +2.71] | 100.00% |
| ✅ | file_to_blackhole | egress throughput | +0.05 | [-6.12, +6.21] | 1.03% |
| ✅ | dogstatsd_string_interner_8MiB_100k | ingress throughput | +0.00 | [-0.01, +0.02] | 5.14% |
| ✅ | trace_agent_msgpack | ingress throughput | +0.00 | [-0.00, +0.00] | 0.95% |
| ❌ | trace_agent_json | ingress throughput | +0.00 | [-0.01, +0.01] | 0.00% |
| ❌ | dogstatsd_string_interner_64MiB_1k | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_8MiB_100 | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_8MiB_10k | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_8MiB_50k | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_64MiB_100 | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_128MiB_1k | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | tcp_dd_logs_filter_exclude | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_8MiB_1k | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | dogstatsd_string_interner_128MiB_100 | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | uds_dogstatsd_to_api | ingress throughput | +0.00 | [-0.00, +0.00] | 0.00% |
| ❌ | file_tree | egress throughput | -0.21 | [-2.00, +1.58] | 15.43% |
| ❌ | tcp_syslog_to_blackhole | ingress throughput | -0.69 | [-0.76, -0.62] | 100.00% |
tasks/components.py
Outdated
| @@ -379,24 +379,32 @@ def lint_fxutil_oneshot_test(_): | |||
| if str(file).endswith("_test.go") or str(file).endswith("main.go") or str(file).endswith("main_windows.go"): | |||
There was a problem hiding this comment.
Can str(file).endswith("main.go") or str(file).endswith("main_windows.go") be removed by using fxutil.TestRun?
There was a problem hiding this comment.
Yes, indeed, that increases coverage for a couple of additional files.
dustmop
force-pushed
the
dustin.long/lint-fx-run
branch
from
20616b9 to
527e6f4
Compare
| @@ -43,3 +44,7 @@ func TestTagsSetup(t *testing.T) { | |||
| assert.Subset(t, metricAgent.GetExtraTags(), allTags) | |||
| assert.Subset(t, logs.GetLogsTags(), allTags) | |||
| } | |||
|
|
|||
| func TestFxApp(t *testing.T) { | |||
There was a problem hiding this comment.
These I think make sense staying this way because they are specifically testing the usage of fx dependencies.
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestFxRun(t *testing.T) { |
| "github.com/DataDog/datadog-agent/pkg/util/fxutil" | ||
| ) | ||
|
|
||
| func TestFxRun(t *testing.T) { |
| @@ -377,7 +377,8 @@ def lint_fxutil_oneshot_test(_): | |||
| for folder in folders: | |||
| folder_path = pathlib.Path(folder) | |||
| for file in folder_path.rglob("*.go"): | |||
| if str(file).endswith("_test.go") or str(file).endswith("main.go") or str(file).endswith("main_windows.go"): | |||
| # Don't lint test files | |||
dustmop
force-pushed
the
dustin.long/lint-fx-run
branch
from
527e6f4 to
73f9fa9
Compare
Serverless Benchmark Results
tl;dr
What is this benchmarking?The The benchmark is run using a large variety of lambda request payloads. In the charts below, there is one row for each event payload type. How do I interpret these charts?The charts below comes from The benchstat docs explain how to interpret these charts.
Benchmark stats |
What does this PR do?
Improve the existing linter check to ensure that users of fx.Run and fx.OneShot have corresponding unit tests. These unit tests should verify that the fx dependencies are correctly fulfilled by using fx. This is done by checking for calls to fxutil.TestOneShot, fxutil.TestOneShotSubcommand, and fxutil.Run.
Motivation
Ensure that fx.dependencies are fulfilled, preventing future breakages.
Additional Notes
Possible Drawbacks / Trade-offs
Describe how to test/QA your changes
Reviewer's Checklist
Triagemilestone is set.major_changelabel if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.changelog/no-changeloglabel has been applied.qa/skip-qalabel is not applied.team/..label has been applied, indicating the team(s) that should QA this change.need-change/operatorandneed-change/helmlabels have been applied.k8s/<min-version>label, indicating the lowest Kubernetes version compatible with this feature.