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

test_util: Add meaningful names to test LogContexts #900

Open
hawkw opened this issue Feb 8, 2024 · 1 comment
Open

test_util: Add meaningful names to test LogContexts #900

hawkw opened this issue Feb 8, 2024 · 1 comment

Comments

@hawkw
Copy link
Member

hawkw commented Feb 8, 2024

Currently, when a test creates a new LogContext, a numeric counter is incremented every time a test creates a new LogContext, and the resultant path will be of the form {test_name}.{pid}.{id}.log. The counter is useful as it ensures that multiple LogContexts can coexist within a single test execution. However, it doesn't really help with identifying what entity in the test created that log file. Instead, this must be inferred by reading the test code to determine the order in which LogContexts are created, which can sometimes be challenging --- especially when there's a bunch of test support code spinning up multiple Dropshot servers.

It would be nice if there was a way to add a human-readable test component identifier to these log paths, like "server" or "client". The counter could still be used in addition to such an additional identifier to ensure all paths are unique.

@davepacheco
Copy link
Collaborator

Hmm. When this was created, the intent was to put the actual test name there. In practice, it's quite often not that anyway because:

  • someone copied and pasted the LogContext::new from a different test
  • someone used a slightly different name than the name of the function (I see this a lot)
  • in the best of cases, it's still only the function name, but Rust tests are really identified by the full module path too, even though I don't think we probably want those in the filename here

I'm wondering if we should just treat this as an "arbitrary caller-provided label", which is really what it is anyway. Then change the callers to provide better names.

Related but tangential: we've long wanted a #[log_context] kind of test macro sort of like nexus_test but that gives your test function a LogContext and also cleans it up on success. If we had that, the logs could more reliably match the test name. But I still think it makes more sense to think of the name as a log label and not "the name of the test being run".

What do folks think?

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

No branches or pull requests

2 participants