Update Test.traits setter to filter applicable values rather than crashing when non-applicable values are encountered

[gmedori]

Prior to this PR, when a developer would define their own SuiteTrait and provide an implementation for isRecursive that resolved to true, the test process would crash unless they also had the trait conform to TestTrait. This happened because we consider a non-suite Test having a SuiteTrait to be invalid, so we placed a precondition failure in the setter for Test.traits. When we would recursively attempt to apply the trait, it would do so for all sub-suites and tests, tripping the precondition.

While this is technically correct behavior, the failure mode is a poor experience that doesn't adequately communicate what went wrong. Here's an example of the log when the crash happens:

Build complete! (0.79s)
Test Suite 'All tests' started at 2026-02-12 17:40:55.984.
Test Suite 'All tests' passed at 2026-02-12 17:40:55.985.
         Executed 0 tests, with 0 failures (0 unexpected) in 0.000 (0.002) seconds
error: Process 'swiftpm-testing-helper --test-bundle-path .build/arm64-apple-macosx/debug/recursive-suite-traitsPackageTests.xctest/Contents/MacOS/recursive-suite-traitsPackageTests .build/arm64-apple-macosx/debug/recursive-suite-traitsPackageTests.xctest/Contents/MacOS/recursive-suite-traitsPackageTests --testing-library swift-testing' exited with unexpected signal code 5

To avoid this crash and to allow for previously infeasible trait configurations, this change updates the Test.traits setter to:

1. Ditch the precondition failure
2. Instead, only actually set valid values

As a short cookbook, we then have the following:

As a developer I want to…	Recipe
… write a trait that only applies to a suite and its sub-suites	Have your trait conform to SuiteTrait and set isRecursive to true (the previous crash scenario). The nested tests will ignore the trait when we attempt to set it on them.
… write a trait that applies to a suite, its sub-suites, and tests	Have your trait conform to both SuiteTrait and TestTrait, and set isRecursive to true. With the extra conformance, nested tests will pick up the trait.

Resolves #1048

Checklist:

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

[grynspan]

Works for me. I'd like at least one of my colleagues to also review before we merge it. :)

[grynspan]

Oh, we need some test coverage!

[gmedori]

Good shout! I'm also adding some test coverage over in #1531. I'll get cooking on those.

[stmontgomery]

Thank you @gmedori for taking this on!

I recall @grynspan proposing this solution once in the past, and back then, I raised a concern that there was one potential use case that this solution would make impossible:

As a developer I want too…
…write a trait that can be explicitly applied to a suite, will be inherited by its sub-suites and tests, but cannot be explicitly applied to a test.

It’s fair to ask whether that's a use case that matters or is useful. So far, the only speculative use case I have come up with is a trait which only conforms to SuiteTrait and TestScoping and is written such that it performs some more expensive scoping work once for the entire suite, but then also does some other, cheaper work once per child test. The author of such a trait might see it as useful to expressly prohibit it from being written explicitly on a test, forcing it to always be written on a suite even though it will inherit to everything.

Some time has passed since TestScoping was introduced though and this need hasn't come up organically, so I think we can reasonably conclude that use case isn't that important. I did want to at least describe it here for awareness in case it comes up in the future though. I'll keep thinking about this a bit more but at the moment I'm inclined to proceed with this approach (and finally put this issue to rest!)

[grynspan]

@gmedori actually independently raised that question with me in DMs. As I replied there:

I would say this behaviour is unintentional but I don't think we designed for it since this would be a broken trait previously.

[gmedori]

@stmontgomery thanks for the thoughtful comment!

So far, the only speculative use case I have come up with is a trait which only conforms to SuiteTrait and TestScoping and is written such that it performs some more expensive scoping work once for the entire suite, but then also does some other, cheaper work once per child test.

Yup as Jonathan said, this was the exact scenario I thought of that is basically unsupported by this change. This scenario was described in a forum post and was the impetus for the original issue being filed, so I wanted to at least consider it. When I brought it up, I was actually concerned that where we used to crash, now we just silently ignore. But I convinced myself that the behavior merely matches the semantics of the declaration. That is, if you make your trait conform to SuiteTrait, then it applies to suites. If you make it conform to TestTrait then it applies to tests. And that just kind of makes sense, and no longer appears to me as "failing silently".

When reading the original forum post, it seemed like the original issue author brought it up not as a production issue, but rather an educational one. And if we haven't seen more concern/discussion about it, I'd agree that this use case is unlikely to be important, which makes me more bullish on this approach.

[grynspan]

If we think there's a practical issue here, we could generate a .apiMisused issue (with .warning severity, I think) about it when we detect it has happened.

[gmedori]

Yeah that was what I was thinking when I posted #1048 (comment) in the original issue. The more that I think about it, though, the more that I'm feeling that the thing we want to detect is not really detectable. Specifically, this is the scenario that we would consider "invalid":

struct RecursiveTrait: SuiteTrait, TestScoping { // <-- ⚠️ No TestTrait conformance
  let isRecursive: Bool = true

  func provideScope(for test: Test, testCase: Test.Case?, performing function: () async throws -> Void) async throws {
    if test.isSuite {
      // 1. Do expensive work before the suite
      // ...
      // 2. Then run the test suite
      try await function()
    } else {
      // Do cheap work before each test
      // ❌ Will never happen because this trait is never applied to tests
    }
  }

  func scopeProvider(for test: Test, testCase: Test.Case?) -> RecursiveTrait? {
    return Self() // Provide a non-nil value always, regardless of suite or test
  }
}

I'm still a bit new to all this so please correct me if I'm missing something, but my understanding is that it's perfectly valid to have a SuiteTrait that conforms to TestScoping without also conforming to TestTrait. The issue is when, at runtime, the developer might want to do something when test.isSuite == false, not realizing it won't fire without the trait conforming to TestTrait and we can't really detect that.

...

Can we? (⚠️ WARNING: Idle musings follow below, feel free to ignore)

So I'll preface this by saying I don't think the juice is worth the squeeze, but purely as a thought exercise, we could build in some sort of alerting mechanism inside of the getter for Test.isSuite that we prime right before we're about to call provideScope. Then we check if it was tripped on any traits that conform to the types we care about.

But that sounds super complex, hacky, and fragile. Furthermore the most refined signal we could get is whether isSuite was accessed, not whether it was negated, part of an if/else, or any information that would tell us how the user branched. We would effectively be saying "don't check isSuite inside of provideScope", which I don't think is fair.