ferruzzi commented on code in PR #43340:
URL: https://github.com/apache/airflow/pull/43340#discussion_r1824899538
##########
tests/core/test_otel_logger.py:
##########
@@ -356,3 +356,116 @@ def test_timer_start_and_stop_manually_send_true(self,
mock_time, metrics_consis
self.meter.get_meter().create_observable_gauge.assert_called_once_with(
name=full_name(name), callbacks=ANY
)
+
Review Comment:
Taking a closer look at the tests cases, the new `test_incr_counter` and
`test_decr_counter` add testing for a new metric with a count, which is new
ground not covered by the existing tests. Nice catch on those!
Could you please move those into the same "block" as their cousins so all
the incr and decr tests are together, rename them to match the existing
convention (ie `test_incr_new_metric_with_count` ), and have them use the
`name` fixture like the others?
The gauge and timer ones I'm not sure what is being tested now that wasn't
already covered. What am I missing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]