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

Convoluted hook testing infra #552

Open
lorenzwalthert opened this issue Mar 28, 2024 · 0 comments
Open

Convoluted hook testing infra #552

lorenzwalthert opened this issue Mar 28, 2024 · 0 comments

Comments

@lorenzwalthert
Copy link
Owner

Is your feature request related to a problem? Please describe.
Convoluted testing infra

Describe the solution you'd like

In general, I think run_test is very convoluted, since std_err (as documented in run_test + me reading the source code) can be

  • a string to signal we expect a certain error message (asserted with matching std err against the expected error message in case).
  • NA to signals we don't expect the file passed to the hook test to change (asserted with comparing file contents before and after).
  • NULL (the default) as a shortcut to avoid typing out expect_success since usually when you expect something in std err, you probably expect the hook to fail. The signature for the reader's convenience:
    run_test(
      std_err = NULL,
      expect_success = is.null(std_err),
      ...
    )

I think to untangle this, we first have to fix run_test_impl() and reduce cognitive overload / multiple meaning on the arguments std_err and std_out :

  • use std_err and std_out only for message matching
  • leverage read_only argument recently added to decide if file content should be the same or not (instead of relying std_err = NA).
  • This might be enough keep the std_err = NULL shortcut without being too confusing.

Describe alternatives you've considered

Not sleep anymore at night since testing infra is messed up 😸

Additional context

Came up in https://github.com/lorenzwalthert/precommit/pull/551/files/b8f72d26d158d86eee8aad887db05ea06f622399#r1541574615

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

1 participant