You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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, sincestd_err
(as documented inrun_test
+ me reading the source code) can beNA
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 outexpect_success
since usually when you expect something in std err, you probably expect the hook to fail. The signature for the reader's convenience:I think to untangle this, we first have to fix
run_test_impl()
and reduce cognitive overload / multiple meaning on the argumentsstd_err
andstd_out
:std_err
andstd_out
only for message matchingread_only
argument recently added to decide if file content should be the same or not (instead of relyingstd_err = NA
).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
The text was updated successfully, but these errors were encountered: