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

move trace_util test helpers behind a feature flag #461

Merged

Conversation

ekump
Copy link
Contributor

@ekump ekump commented May 29, 2024

What does this PR do?

  1. Moves test helper function for trace-utils behind a feature-flag called test-utils.
  2. Move poll_for_mock_hit and create_send_data functions into test-utils.

To limit trace_test_utils to only being used in tests your Cargo.toml file should look like:

[dependencies]
datadog-trace-utils = { path = "../trace-utils" }

[dev-dependencies]
datadog-trace-utils = { path = "../trace-utils", features = ["test-utils"] }

Motivation

Test code shouldn't be available for use outside of tests.

Because the test-utils need to be used in tests both within trace-utils and outside of trace-utils it limited possible solutions:

  • Simply wrapping trace_test_utils with #[cfg(test)] at the module export level doesn't work since it prevents these functions from being imported in tests in other crates.
  • Creating a separate "datadog_test_utils" crate creates errors in tests inside trace-utils when the helper functions return objects defined in trace-utils. For example, the Rust compiler thinks there is a type mismatch between datadog_test_utils::send_data::SendData (returned by the create_send_data function) and the SendData type in the send_data.rs unit tests even though they are the same type. No matter how I defined imports, I couldn't get this type mismatch to go away.

How to test the change?

Tests that use these helpers still pass.

@ekump ekump changed the title Ekump/apmsp 1153 move trace util helpers to separate crate ekump/apmsp 1153 move trace util helpers to separate crate May 29, 2024
@ekump ekump changed the title ekump/apmsp 1153 move trace util helpers to separate crate move trace util test helpers to separate crate May 29, 2024
@ekump ekump changed the title move trace util test helpers to separate crate move trace_util test helpers to separate crate May 29, 2024
@ekump ekump force-pushed the ekump/APMSP-1153-move-trace-util-helpers-to-separate-crate branch from ead932e to a59150f Compare May 29, 2024 17:42
@codecov-commenter
Copy link

codecov-commenter commented May 29, 2024

Codecov Report

Attention: Patch coverage is 95.45455% with 7 lines in your changes missing coverage. Please review.

Project coverage is 68.91%. Comparing base (5b9ba35) to head (615233d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #461      +/-   ##
==========================================
- Coverage   68.93%   68.91%   -0.03%     
==========================================
  Files         193      193              
  Lines       25733    25732       -1     
==========================================
- Hits        17739    17732       -7     
- Misses       7994     8000       +6     
Components Coverage Δ
crashtracker 19.34% <ø> (ø)
datadog-alloc 98.76% <ø> (ø)
data-pipeline 51.45% <ø> (ø)
data-pipeline-ffi 0.00% <ø> (ø)
ddcommon 85.89% <ø> (ø)
ddcommon-ffi 74.15% <ø> (ø)
ddtelemetry 56.09% <ø> (ø)
ipc 81.69% <ø> (ø)
profiling 77.97% <ø> (ø)
profiling-ffi 60.00% <ø> (ø)
serverless 0.00% <ø> (ø)
sidecar 36.49% <100.00%> (-0.61%) ⬇️
sidecar-ffi 0.00% <ø> (ø)
spawn-worker 54.98% <ø> (ø)
trace-mini-agent 69.33% <ø> (ø)
trace-normalization 97.79% <ø> (ø)
trace-obfuscation 95.74% <100.00%> (ø)
trace-protobuf 33.33% <ø> (ø)
trace-utils 90.94% <95.20%> (-0.24%) ⬇️

@ekump ekump force-pushed the ekump/APMSP-1153-move-trace-util-helpers-to-separate-crate branch 3 times, most recently from e2cad81 to c992e44 Compare June 3, 2024 19:53
ekump added 2 commits June 3, 2024 15:57
To reduce the possibility of test helper functions being used in
non-test code. Also, move create_send_data and poll_for_mock_hit
functions into trace_test_utils.
@ekump ekump force-pushed the ekump/APMSP-1153-move-trace-util-helpers-to-separate-crate branch from c992e44 to 615233d Compare June 3, 2024 19:57
@ekump ekump marked this pull request as ready for review June 3, 2024 20:09
@ekump ekump requested review from a team as code owners June 3, 2024 20:09
@ekump ekump changed the title move trace_util test helpers to separate crate move trace_util test helpers behind a feature flag Jun 3, 2024
@ekump ekump merged commit 56b1f7f into main Jun 4, 2024
27 checks passed
@ekump ekump deleted the ekump/APMSP-1153-move-trace-util-helpers-to-separate-crate branch June 4, 2024 13:13
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.

4 participants