swift-testing icon indicating copy to clipboard operation
swift-testing copied to clipboard

Promote exit tests to API

Open grynspan opened this issue 1 year ago • 30 comments

This PR promotes exit tests to API, pending approval of the proposal at https://github.com/swiftlang/swift-evolution/pull/2718.

View the full proposal here.

Checklist:

  • [x] Code and documentation should follow the style of the Style Guide.
  • [x] If public symbols are renamed or modified, DocC references should be updated.

grynspan avatar Apr 02 '24 17:04 grynspan

Rebased.

grynspan avatar Jul 12 '24 15:07 grynspan

Rebased on #615.

grynspan avatar Aug 15 '24 23:08 grynspan

Nice. Three notes:

  1. You can probably support exit tests on platforms that don't let you spawn, because there is always(?) a test driver program running on a host system like a Mac. I outlined how this could work for googletest and CMake here: instead of looping over all the tests in a process on the target platform, you loop over the expected-to-fail ones on the host and launch a separate target process for each one.
  2. I'm not familiar enough with what you're doing to know whether you've covered this case already, but it's sometimes important to be able to verify stdout/stderr contents in addition to detecting abnormal exit. I hope your design allows that.
  3. With GTest I found it very inconvenient that platforms weren't uniform w.r.t. what I could detect in an exit test. I had to make uniformity macros that allowed me to write one test for all platforms and verify as much as possible. They are described (and linked) here. A better library would provide these facilities OOTB.

dabrahams avatar Sep 24 '24 18:09 dabrahams

@dabrahams Please read the draft pitch. :) 1. and 2. are covered in it. It sounds like 3. is sufficiently covered I think? .signal() is not supported on Windows because Windows itself only has emulated signal support. I know how we can fix that (by replacing the default signal handlers on Windows that call exit(3) with something that communicates across the "back channel") but I've punted that from the initial proposal for the sake of complexity.

grynspan avatar Sep 24 '24 18:09 grynspan

I skimmed the pitch; I didn't see coverage for 1; what I saw was

The need for exit tests on other platforms is just as strong as it is on the supported platforms (macOS, Linux, and Windows). These platforms do not support spawning new processes, so a different mechanism for running exit tests would be needed.

emphasis mine. I read that as meaning “we don't have an answer for these platforms.”

What I see about 2) is an explicit statement that it isn't supported.

It sounds like 3 is not sufficiently covered, since the Windows signal detection case is one of the ones for which I had to build a workaround manually, and you say you're punting it 'till later.

Maybe by “covered” you just mean “discussed.” In that case I don't see much discussion of strategies for handling these things, so I thought my comment would be useful. If it wasn't, feel free to ignore it.

dabrahams avatar Sep 24 '24 18:09 dabrahams

I skimmed the pitch; I didn't see coverage for 1; what I saw was

The need for exit tests on other platforms is just as strong as it is on the supported platforms (macOS, Linux, and Windows). These platforms do not support spawning new processes, so a different mechanism for running exit tests would be needed.

emphasis mine. I read that as meaning “we don't have an answer for these platforms.”

I'm sure that, as a former Apple employee, you can appreciate that my ability to discuss iOS et al. is limited. 🙂

What I see about 2) is an explicit statement that it isn't supported.

It's listed as a future direction. That doesn't mean it can't be done, just that it's not part of this pitch.

It sounds like 3 is not sufficiently covered, since the Windows signal detection case is one of the ones for which I had to build a workaround manually, and you say you're punting it 'till later.

If you want to detect specifically that a Windows process terminated with a specific signal, that's something that needs additional work beyond the scope of this pitch. If you want to detect that a Windows process terminated abnormally, which would include termination due to an unhandled signal, that's supported and covered by the general .failure exit condition.

grynspan avatar Sep 24 '24 18:09 grynspan

To be clear (I'm now reading my initial reply as snarky, which wasn't intentional), I appreciate the feedback @dabrahams. I can certainly amend the pitch to make it clearer we're thinking about these things.

Edit: I added a "future directions" section about Windows signals.

grynspan avatar Sep 24 '24 18:09 grynspan

Windows signals are now handled! #766

grynspan avatar Oct 22 '24 02:10 grynspan

And stdout/stderr will be soon too. #773

grynspan avatar Oct 22 '24 02:10 grynspan

Windows signals are now handled! https://github.com/swiftlang/swift-testing/pull/766

Nice; That puts you at least one step ahead of googletest, IIRC.

dabrahams avatar Oct 22 '24 02:10 dabrahams

@swift-ci test

grynspan avatar Oct 23 '24 18:10 grynspan