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_runner: enable testing panics in mainTerminal #15991

Closed
wants to merge 4 commits into from

Conversation

matu3ba
Copy link
Contributor

@matu3ba matu3ba commented 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,
    1. panic_msg is set as threadlocal, if comptime-detectable capabilities exist, to enable multithreaded processing and user-customized messages,
    1. error.SpawnZigTest is returned to the test_runner.zig
    1. the test_runner spawns a child_process on correct usage
    1. 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 #14351.
Work on #1356.

@matu3ba
Copy link
Contributor Author

matu3ba commented Jun 9, 2023

Question: Is the approach lit? Should I include it like this in Server with test code or are there any concerns?

// testfile.zig

const std = @import("std");
const testing = std.testing;

test "panic" {
    try testing.spawnExpectPanic("test1");
    try testing.expectEqual(@as(u8, 1), @as(u32, 1));
    @panic("test1");
}

test "wrong_panic" {
    try testing.spawnExpectPanic("test1");
    @panic("test2");
}
// Test [2/3] test.wrong_panic... Signal: 6
// Test [2/3] test.wrong_panic... FAIL expected_panic_msg: 'test1', got: 'test2

test "no panic but one was expected" {
    try testing.spawnExpectPanic("test3");
}
// Test [2/3] test.no panic but one was expected... error.SpawnZigTest
// Test [2/3] test.no panic but one was expected... spawning '/home/misterspoon/dev/git/zi/zig/enable_testing_pan
ics/zig-cache/o/4b2b4b4abad5f54a10c57ea01651fa0d/test --test_panic_index 2'
// Test [2/3] test.no panic but one was expected... FAIL term exited, status: 0)
// stdout: ()
// stderr: ()

// unhandled case:
// test "testme" {
//     @panic("123123");
// }
./deb/bin/zig test testfile.zig 
Test [1/3] test.panic... SPAWN '/home/jan/dev/zdev/zig/enable_testing_panics/zig-cache/o/b0b2decbe7ec71ffe70cf209
7d241485/test --test_panic_index 0'
Test [1/3] test.panic... Signal: 6
Test [2/3] test.wrong_panic... SPAWN '/home/jan/dev/zdev/zig/enable_testing_panics/zig-cache/o/b0b2decbe7ec71ffe7
0cf2097d241485/test --test_panic_index 1'
Test [2/3] test.wrong_panic... Signal: 6
Test [2/3] test.wrong_panic... FAIL expected_panic_msg: 'test1', got: 'test2', Signal: 6
Test [3/3] test.no panic but one was expected... SPAWN '/home/jan/dev/zdev/zig/enable_testing_panics/zig-cache/o/
b0b2decbe7ec71ffe70cf2097d241485/test --test_panic_index 2'
Test [3/3] test.no panic but one was expected... FAIL term exited, status: 0)
stdout: ()
stderr: ()
1 passed; 0 skipped; 2 failed.
error: the following test command failed with exit code 1:
/home/jan/dev/zdev/zig/enable_testing_panics/zig-cache/o/b0b2decbe7ec71ffe70cf2097d241485/test

@matu3ba
Copy link
Contributor Author

matu3ba commented Jun 9, 2023

later after design issues resolved or approach clarified:

  • move into function
  • server integration
  • clarify how the server logic should work
    • must store remaining time left for process,
    • must store gid+pid to kill it, if taking too long
    • must regularly check, if already finished
    • double check where list of unfinished tests and status is stored

@matu3ba
Copy link
Contributor Author

matu3ba commented Oct 20, 2023

This is was is hitting x86_64 backend missing feature[s]:

test transitive failure
+- test-behavior transitive failure
|  +- run test behavior-x86_64-windows-gnu-Debug-selfhosted-no-lld transitive failure
|     +- zig test Debug x86_64-windows-gnu 1 errors
+- test-universal-libc transitive failure
   +- run test universal-libc-x86_64-windows-gnu-Debug-selfhosted-no-lld transitive failure
      +- zig test Debug x86_64-windows-gnu 1 errors
D:\a\zig\zig\lib\std\unicode.zig:200:5: error: TODO implement airReduce for x86_64
D:\a\zig\zig\lib\std\unicode.zig:200:5: error: TODO implement airReduce for x86_64

I'll try to rebase to fix it. rebased. The issue persists.

Jan Philipp Hafer and others added 4 commits October 20, 2023 17:02
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.
Also add the signal to the user as additional info.
@andrewrk
Copy link
Member

Closing abandoned pull request (last update 3 months ago, not passing CI checks)

@andrewrk andrewrk closed this Jan 16, 2024
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.

2 participants