Suggestion: Only warn about missing test suite for API token once

[mokagio]

💬 Context

I haven't been able to track why, but one of our projects logs "[BuildkiteTestCollector] error: Failed to upload result, got error: A test suite could not be found with the supplied API token" 678 times when the tests run.

This happens only in that project, as far as I've seen, so I'd say there's something somewhere in how the tests run there that results in several upload attempts.

Still, it might be possible to mitigate this at the library level by only logging the error once, or once per API key used.

📝 Notes

N.A.

👩‍🔧 Technical Design Notes

A refinement might be introducing different logging levels for the library. If logging is verbose, it could log on every failed upload, otherwise only on the first.

🤝 Related

N.A.

[iampatbrown]

This happens only in that project, as far as I've seen, so I'd say there's something somewhere in how the tests run there that results in several upload attempts.

@mokagio Do any of the results actually get sent? That error looks like it would be if an incorrect api key was used which would mean the result wouldn't be able to upload unless the tests were run again with a different api key.

Still, it might be possible to mitigate this at the library level by only logging the error once, or once per API key used.

At a library level this will most likely be fixed by #44. That will upload results in batches of 5000 which should mean you'd only see this error once if you sent 678 results.

[mokagio]

👋 @iampatbrown

@mokagio Do any of the results actually get sent? That error looks like it would be if an incorrect api key was used which would mean the result wouldn't be able to upload unless the tests were run again with a different api key.

No results are sent. I get those errors only locally, on my machine, where there's no API key set. In CI everything works as expected.

At a library level this will most likely be fixed by #44.

🎉 – Fair enough. The chance for multiple logs is still there, but with a batch size of 5k, even the most hardcore codebases should get but a few logs locally.

Still, I'm not sure why my other projects print that warning only once when running the tests locally 🤔

[iampatbrown]

Still, I'm not sure why my other projects print that warning only once when running the tests locally

@mokagio Are they the same error? The error above will occur when an API Key is found but is not valid. Which can only be determined after attempting an upload. If there is no api key or it is empty you will only get one error saying it won't try to upload. Is it possible that project has the API Key set to something? Can it be cleared?

The chance for multiple logs is still there

Yeah true. We could add some logic that essentially stops the uploader after receiving an error like that. It probably just adds a little but if complexity that we'd need to keep in sync with the backend. i.e Ensure the error doesn't change.

[mokagio]

@iampatbrown

Are they the same error?

🤦‍♂️ of course they're not! Thank you for making me realize that.

For some reason I have yet to uncover, the project that floods the log has the token as "" while the other ones correctly set it to nil.

It's odd, because the setup is the same in both projects. Or rather, should be the same. Evidently, there's something different. It's just a matter of finding it.

The investigation continues... 🕵️‍♂️

[iampatbrown]

For some reason I have yet to uncover, the project that floods the log has the token as "" while the other ones correctly set it to nil.
@mokagio Right now the main branch will ignore empty strings as well. A new release should be cut soon which should fix the issue for you.

You could potentially point you main and confirm if you'd like

test-collector-swift/Sources/Core/TestCollector.swift

Line 25 in 4d858f6

if let apiToken = environment.analyticsToken, apiToken != "" {