Skip to content

[TEST] Fix ZipkinExporterTestPeer.ExportJsonIntegrationTest stability#3760

Merged
marcalff merged 5 commits intoopen-telemetry:mainfrom
tushar1977:main
Nov 24, 2025
Merged

[TEST] Fix ZipkinExporterTestPeer.ExportJsonIntegrationTest stability#3760
marcalff merged 5 commits intoopen-telemetry:mainfrom
tushar1977:main

Conversation

@tushar1977
Copy link
Copy Markdown
Contributor

@tushar1977 tushar1977 commented Nov 22, 2025

Fixes #3723

Changes

  • Fix test ZipkinExporterTestPeer.ExportJsonIntegrationTest to expect 1 or 2 exports.
  • This is needed when the batch processor is scheduled after the child span ends, but before the parent span ends.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.92%. Comparing base (16c7067) to head (2abfe1b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3760      +/-   ##
==========================================
- Coverage   89.95%   89.92%   -0.02%     
==========================================
  Files         225      225              
  Lines        7158     7158              
==========================================
- Hits         6438     6436       -2     
- Misses        720      722       +2     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@marcalff
Copy link
Copy Markdown
Member

@tushar1977

Since you seem to be a student, allow me to provide some guidance on how to address this.

First, the original issue needs to be reproduced, to understand the problem.

I have done that, see the comments in #2723

Second, the fix needs to be tested locally to make sure it works, including when the original issue is seen.

With the following code:

  EXPECT_CALL(*mock_http_client, Post(expected_url, _, IsValidMessage(report_trace_id), _, _))

      .Times(Between(1, 2)) /* <-- proposed fix */
      .WillOnce(Return(ByMove(ext::http::client::Result{
          std::unique_ptr<ext::http::client::Response>{new ext::http::client::curl::Response()},
          ext::http::client::SessionState::Response})));

  child_span->End();
  std::this_thread::sleep_for(std::chrono::milliseconds(1000)); /* <-- how to reproduce the bug */
  parent_span->End();

the test still fails, so the fix is incorrect.

[malff@malff-desktop build-otelcpp-abiv1]$ ./exporters/zipkin/zipkin_exporter_test 
Running main() from /builddir/build/BUILD/googletest-release-1.11.0/googletest/src/gtest_main.cc
[==========] Running 4 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 4 tests from ZipkinExporterTestPeer
[ RUN      ] ZipkinExporterTestPeer.ExportJsonIntegrationTest

GMOCK WARNING:
/home/malff/CODE/MARC_GITHUB/opentelemetry-cpp/exporters/zipkin/test/zipkin_exporter_test.cc:159: Actions ran out in EXPECT_CALL(*mock_http_client, Post(expected_url, _, IsValidMessage(report_trace_id), _, _))...
Called 2 times, but only 1 WillOnce() is specified - returning default value.
Stack trace:
terminate called after throwing an instance of 'std::runtime_error'
  what():  
    The mock function has no default action set, and its return type has no default value set.
Aborted (core dumped)

There is more to it.

Last, once you have a fix that works locally when the bug is exposed, you can verify the fix still works in the normal case, by removing the added sleep_for.

Only then will be patch be ready.

What needs to be resolved at this point is Called 2 times, but only 1 WillOnce() is specified

@tushar1977
Copy link
Copy Markdown
Contributor Author

Hi @marcalff
I have fixed and checked this issue in my local (by including the sleep and removing it). Previously, the issue was that i was doing 2 number of times but i was calling WillOnce only once.
So there were 2 approaches to solve this. One was to include WillOnce twice and other was using WillRepeatedly. But the functionByMove will be performed only once. To tackle that, i used Invoke method which creates a fresh object on every call.

@tushar1977
Copy link
Copy Markdown
Contributor Author

Its failing on macOS and maintainer mode. Let me check that

@tushar1977 tushar1977 marked this pull request as ready for review November 24, 2025 13:32
@tushar1977 tushar1977 requested a review from a team as a code owner November 24, 2025 13:32
@marcalff marcalff changed the title Incremented number of POST request calls from 1 to 2 in ZipkinExporterTestPeer.ExportJsonIntegrationTest [TEST] Fix ZipkinExporterTestPeer.ExportJsonIntegrationTest stability Nov 24, 2025
Copy link
Copy Markdown
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the test fix.

@marcalff
Copy link
Copy Markdown
Member

@tushar1977

See edits to the title, and PR comments.

@marcalff marcalff merged commit c06c255 into open-telemetry:main Nov 24, 2025
105 of 114 checks passed
@tushar1977
Copy link
Copy Markdown
Contributor Author

@tushar1977

See edits to the title, and PR comments.

sure i'll make sure of it in future PR's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TEST] Test failure in ZipkinExporterTestPeer.ExportJsonIntegrationTest

2 participants