Fix TestDaemonProxy integration tests#46122
Conversation
thaJeztah
added
status/2-code-review
area/testing
kind/bugfix
thaJeztah
force-pushed
the
fix_daemon_integration_test
branch
from
4c29a87 to
587507e
Compare
| t.Run("reload sanitized", func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| ctx := context.TODO() |
There was a problem hiding this comment.
I added a single ctx here; this should (deliberately) conflict with the testutil.StartSpan(ctx, t) in #45652, so that we don't forget 😃
There was a problem hiding this comment.
Why don't we use context.Background() here?
There was a problem hiding this comment.
Because this one needs to be replaced with the otel one 😂
Either one should be fine I guess
thaJeztah
force-pushed
the
fix_daemon_integration_test
branch
from
587507e to
5c15521
Compare
|
@cpuguy83 PTAL well ... guess you wrote the code, but to check if my "rebase" without OTEL LGTY 😅 |
Allows tests to report their proxy settings for easier troubleshooting on failures. Signed-off-by: Brian Goff <[email protected]>
thaJeztah
force-pushed
the
fix_daemon_integration_test
branch
from
5c15521 to
2586414
Compare
| } | ||
| } | ||
|
|
||
| // PollCheckLogs is a poll.Check that checks the daemon logs for the passed in string (`contains`). |
There was a problem hiding this comment.
The docs are not right here, there is no “contains” parameter
| scanner := bufio.NewScanner(rdr) | ||
| for scanner.Scan() { | ||
| if match(scanner.Text()) { | ||
| return true, scanner.Text(), nil |
There was a problem hiding this comment.
Might be nice in a followup to have the context around the scanned text, kinda like grep -C to help debugging a failure
I noticed this was always being skipped because of race conditions checking the logs. This change adds a log scanner which will look through the logs line by line rather than allocating a big buffer. Additionally it adds a `poll.Check` which we can use to actually wait for the desired log entry. Signed-off-by: Brian Goff <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
Also fixes up some cleanup issues. Signed-off-by: Brian Goff <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
fix_daemon_integration_test
branch
from
2586414 to
1a51898
Compare
|
There was a test that failed earlier; But CI kept running, until it timed out; Aaaaaand.... I just now realized the reason for that was starting me right in the face, and is in the logs; LOL, so I wonder what part of our CI that is, and if we need to use some other way to make the Windows CI stop on a failure. |
|
Either way, CI is green now, so bringing this one in |
3 participants
Trying to extract the test fixes from #45652, without the OTEL changes, so that we have the ability to backport those to other branches
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)