Skip to content

Convoluted hook testing infra #552

Description

@lorenzwalthert

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions