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

lib: add panic test runner #14351

Closed
wants to merge 6 commits into from
Closed

Conversation

matu3ba
Copy link
Contributor

@matu3ba matu3ba commented Jan 17, 2023

Panic test runner starts itself as child process with the test function
index representing a test block. The custom panic handler compares a
global previously written string. If the panic message is expected, the
success count is increased and the next test block is run. On failure
the parent process prints collected results, before it terminates.

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

Depends on #14152.
Closes #1356.

@matu3ba
Copy link
Contributor Author

matu3ba commented Jan 17, 2023

TODO:

  • As far as I understand, signal handling code by Kernel does not lead to panic being called. Clarify desired semantics: Do we want to add the complexity to check "all exit code paths" from a program once "expectPanic" has been called?
    A: Document all global state under our control in test runner.
  • panic testing requires process spawn due to noreturn of custom panic handler or "recovery code" by the user: Clarify, if we should add extra complexity to compiler to provide enable non-spawning ones to use panic checks (requires panic to be returnable) briefly mentioned here Proposal: std.testing.expectIllegalBehavior #5917 (comment):
- 1. use user provided "restoring to safe process state" code + longjmp
- 2. add to all functions under test a quasi-reserved + implicitly added (propagating) error.panic and let user deal with "restoring to safe process state"

A: after thinking about it for some time: No.

  • cleanup of code
  • Document process spawning + IPC as requirement to keep things simple and test behavior = code behavior.
  • Document all global OS state under our control in test runner.
  • Think of and document panic test naming convention
  • test runner and build runner coordinate over a protocol via IPC, so implement it
  • Think of + document best practice for code reusage: Do not use @panic, unless your code path is strictly intended for non-embedded, because downstream users can not efficiently test error handling of their code on embedded devices. If you must use @panic or there is no reasonable alternative, write sufficient tests and documentation.

Separate PR[s]:

  • os.posix for the signal mask(s) to clarify that they are not unifiable
  • fix default implementation of panic handler
  • make running panic signal safe

@matu3ba

This comment was marked as resolved.

Jan Philipp Hafer added 6 commits April 29, 2023 01:16
This PR simplifies the setup code and handling of non-standard file
descriptors and handles for Windows and Posix with main focus on pipes.

Portable pipes default to pipe2 with CLOEXEC on Posix and disabled handle
inhertance on Windows except shortly before and after the creation of
the "child process".

Leaking of file desriptors on Posix
1. CLOEXEC does not take immediately effect with clone(), but after the
setup code for the child process was run and a exec* function is executed.
External code may at worst be long living and never do exec*.
2. File descriptors without CLOEXEC are designed to "leak to the child",
but every spawned process at the same time gets them as well.

Leaking of handles on Windows
1. File leaking on Windows can be fixed with an explicit list approach
as suggested in ziglang#14251, which might require runtime probing and
allocation of the necessary process setup list. Otherwise, it might
break on Kernel updates.
2. The potential time for leaking can be long due trying to spawn on
multiple PATH and PATHEXT entries on Windows.
Panic test runner starts itself as child process with the test function
index representing a test block. The custom panic handler compares a
global previously written string.  If the panic message is expected, the
success count is increased and the next test block is run. On failure
the parent process prints collected results, before it terminates.

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

Depends on ziglang#14152.
Closes ziglang#1356.
This leaks implementation details from the test runner into libstd to
access the to be spawned test block in a new process and parse its
output or return status.

Future TODO: test blocks can observe their own index fn argument.
matu3ba added a commit to matu3ba/zig that referenced this pull request May 8, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
matu3ba added a commit to matu3ba/zig that referenced this pull request May 14, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
matu3ba added a commit to matu3ba/zig that referenced this pull request Jun 3, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
matu3ba added a commit to matu3ba/zig that referenced this pull request Jun 9, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
matu3ba added a commit to matu3ba/zig that referenced this pull request Jun 9, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
@matu3ba
Copy link
Contributor Author

matu3ba commented Jun 9, 2023

Superseded by #15991, because the spawned process can not be handled by the server.

@matu3ba matu3ba closed this Jun 9, 2023
@matu3ba matu3ba deleted the panic_testrunner branch June 9, 2023 23:23
matu3ba added a commit to matu3ba/zig that referenced this pull request Jun 13, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
andrewrk pushed a commit to matu3ba/zig that referenced this pull request Oct 19, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
matu3ba added a commit to matu3ba/zig that referenced this pull request Oct 20, 2023
The user can use std.testing.spawnExpectPanic() in a test to spawn a
child process, which must panic or the test fails.
Internally,
- 1. is_panic_parentproc is set from the cli arguments for simple
reproduction of both test spawn and panic behavior,
- 2. panic_msg is set as threadlocal, if comptime-detectable capabilities
exist, to enable multithreaded processing and user-customized messages,
- 3. error.SpawnZigTest is returned to the test_runner.zig
- 4. the test_runner spawns a child_process on correct usage
- 5. the child_process expected to panic executes only one test block

This means, that only one @Panic is possible within a test block and that
no follow-up code after the @Panic in the test block can be run.

This commit does not add the panic test capability to the server yet,
since there are open design questions how many processes should be
spawned at the same time and how to manage time quotas to prevent
unnecessary slowdowns.

Supersedes ziglang#14351.
Work on ziglang#1356.
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

Successfully merging this pull request may close these issues.

Proposal: Add a way for a test to expect a panic
1 participant