open-telemetry / opentelemetry-rust

The Rust OpenTelemetry implementation
https://opentelemetry.io
Apache License 2.0
1.82k stars 424 forks source link

Use parameterization to simplify tests #1552

Open hdost opened 7 months ago

hdost commented 7 months ago

Consider the use of parameterized tests for scenarios where only the inputs and expected outputs differ, to reduce redundant test code. Rust doesn't support it directly, but there are external crates providing this option e.g., - https://crates.io/crates/parameterized_test , or we can create a custom macro as suggested here - https://stackoverflow.com/questions/34662713/how-can-i-create-parameterized-tests-in-rust/

Not related to this PR, for future :)

Originally posted by @lalitb in https://github.com/open-telemetry/opentelemetry-rust/pull/1516#issuecomment-1946900301

sg126 commented 6 months ago

Hi, would I be able to take a look at this if that's possible? Don't want to interfere if someone else has already decided to hop on this.

TommyCpp commented 6 months ago

Hi, would I be able to take a look at this if that's possible? Don't want to interfere if someone else has already decided to hop on this.

I don't think anyone else is working on it. Assigned it to you. Thanks for the help!

sg126 commented 6 months ago

Is there a good place that we could put parametrized tests as a starting point or would it be preferred to have this in all applicable areas?

cijothomas commented 6 months ago

Is there a good place that we could put parametrized tests as a starting point or would it be preferred to have this in all applicable areas?

https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-sdk/src/metrics/mod.rs This would be a good place to begin

sg126 commented 6 months ago

Perfect, thank you. I'll get started there

cijothomas commented 4 months ago

@sg126 hey just checking if you are still planning to work on this! Let us know if you need help with anything to unblock!

sg126 commented 3 weeks ago

Hey! Super sorry for the delay, I've been off due to things taking up my time. I will unassign myself. Sorry for the inconvenience!

PvVismitha commented 1 week ago

@cijothomas can I take up this issue

cijothomas commented 1 week ago

@cijothomas can I take up this issue

Yes sure! Thanks in advance.