Skip to content

Conversation

@JoukoVirtanen
Copy link
Contributor

@JoukoVirtanen JoukoVirtanen commented Nov 1, 2025

Description

The TestRepeatedNetworkFlowWithZeroAfterglowPeriod fails frequently with errors such as

=== RUN   TestRepeatedNetworkFlowWithZeroAfterglowPeriod/TestRepeatedNetworkFlow
    expect_conn.go:72: 
        	Error Trace:	/home/runner/work/collector/collector/integration-tests/pkg/mock_sensor/expect_conn.go:72
        	            				/home/runner/work/collector/collector/integration-tests/suites/repeated_network_flow.go:114
        	Error:      	timed out
        	Test:       	TestRepeatedNetworkFlowWithZeroAfterglowPeriod/TestRepeatedNetworkFlow
        	Messages:   	found 4 connections (expected 3)

This PR does not fix that, but improves logging for this test. A partial fix is in another PR #2642. This PR also makes it so that the intervals between the curls will be more precise. Currently a sleep is used between curls. This means that the time between the start of when two curl commands are executed is actually the sleep time plus the time taken by the curl command. This PR makes it so that the time between curl commands is actually the intended time.

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

Ran locally and checked the logs of the qa-schedule-curls container.

Saw the following in the logs

[18:27:37.387] Executing curl (Meta: 1/1, Iter: 1/3)
Curl timing: start=[18:27:37.387], end=[18:27:37.390], duration=0.003s
[18:27:40.389] Executing curl (Meta: 1/1, Iter: 2/3)
Curl timing: start=[18:27:40.389], end=[18:27:40.401], duration=0.012s
[18:27:43.390] Executing curl (Meta: 1/1, Iter: 3/3)
Curl timing: start=[18:27:43.390], end=[18:27:43.402], duration=0.012s
Script finished successfully.

The precision of time.sleep is about 1-10ms so the timings still are not precise to the millisecond.

@JoukoVirtanen JoukoVirtanen requested a review from a team as a code owner November 1, 2025 18:07
@codecov-commenter
Copy link

codecov-commenter commented Nov 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.60%. Comparing base (55be868) to head (dcb8376).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2641   +/-   ##
=======================================
  Coverage   27.60%   27.60%           
=======================================
  Files          95       95           
  Lines        5422     5422           
  Branches     2523     2523           
=======================================
  Hits         1497     1497           
  Misses       3213     3213           
  Partials      712      712           
Flag Coverage Δ
collector-unit-tests 27.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

// Invokes the client container and runs the curls
containerID, err = s.Executor().StartContainer(config.ContainerStartConfig{Name: "nginx-curl", Image: scheduled_curls_image, Command: []string{"python3", "/usr/bin/schedule-curls.py", numMetaIter, numIter, sleepBetweenCurlTime, sleepBetweenIterations, serverAddress}})
s.Require().NoError(err)
s.ClientContainer = containerID[0:12]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously the container was started with a sleep command. That meant that no logs were obtained. Now that the container is started with the python script, we get logs for the python script.

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.

2 participants