Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(tests): Enable verbose output for tests #16459

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zak-pawel
Copy link
Collaborator

Summary

To see the test results for the logger package.

Checklist

  • No AI generated code was used in this PR

@telegraf-tiger telegraf-tiger bot added the chore label Feb 1, 2025
@zak-pawel
Copy link
Collaborator Author

It seems to me that something is wrong with the logger package and/or the gotestsum tool.

Here is the test of the logger package on Linux using go test:
image

As we can see, 21 tests intended for Linux were conducted across three different files (logger_test.go, structured_logger_test.go, and text_logger_test.go).
There are a total of 21 test methods, all of which passed, so everything looks fine.

Now, testing the same package (also on Linux) using gotestsum:
image

We see that only 4 tests were conducted - both from logger_test.go and the first 2 from structured_logger_test.go.
All 4 tests passed, and the test itself ends "successfully."

But what about the remaining 17 tests?

(I ran the tests on a fresh master branch without any modifications.)

@zak-pawel
Copy link
Collaborator Author

What about CI?

Let's take the latest nightly run on the master branch as an example: https://app.circleci.com/pipelines/github/influxdata/telegraf/24207/workflows/09a89f10-312d-4e77-8681-7123de0b805a

Lets analyze the test logs from all platforms (test-go-windows, test-go-mac, test-go-linux-386, test-go-linux).

Only in test-go-windows we see any information about tests in the logger package, and that’s just that 2 tests were skipped because: "Skipping integration test in short mode."
(Interestingly, both of these tests are from event_logger_test.go, which is Windows-specific. Since we never run integration tests on Windows, these tests are never actually executed.)
image

For the remaining platforms, there is no trace that the logger package even exists or has any tests.

The same issue with the logger package not being tested occurs in non-nightly tests on the master branch (https://app.circleci.com/pipelines/github/influxdata/telegraf/24172/workflows/fee4ffb2-da00-4254-9fd4-05739e8f4ff3) and in tests triggered by PRs (https://app.circleci.com/pipelines/github/influxdata/telegraf?branch=pull%2F16423).

(In both cases, the test-integration job is also executed, but there is no information about the logger package being tested there either.)

@zak-pawel
Copy link
Collaborator Author

In this PR, I'm making only one change: adding the --format standard-verbose flag to gotestsum.
https://github.com/gotestyourself/gotestsum?tab=readme-ov-file#output-format

Let's look at the CI https://app.circleci.com/pipelines/github/influxdata/telegraf/24201/workflows/acc1ff07-45c3-4ddd-b3c1-b7d28569c13e:
image

Finally, some information about the logger package appears. For all platforms, only 4 tests are executed in the logger package (the same ones I mentioned earlier).

When I run tests for this package in GoLand, I get the same situation - 4 tests pass, but the job is automatically "terminated."
image

@zak-pawel
Copy link
Collaborator Author

@srebhan What do you think?

@srebhan
Copy link
Member

srebhan commented Feb 4, 2025

@zak-pawel TBH I'm not so much in favor of adding more output for cases where the tests pass. However, I'm really concerned about the tests not being run... Or are they? If I do gotestsum --format standard-verbos -- -v -race -short I see all 21 tests are running, while without the -v I only see the 4 ones you mention...

So what is the purpose of adding the flag? Testing? Or do you want to also do it "in production"?

@zak-pawel
Copy link
Collaborator Author

@srebhan

So what is the purpose of adding the flag? Testing?

Testing, just want to understand what is going on with this package.

If I do gotestsum --format standard-verbos -- -v -race -short I see all 21 tests are running, while without the -v I only see the 4 ones you mention...

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants