-
Notifications
You must be signed in to change notification settings - Fork 104
Introduce conditional trait that support "all must be met" and an "any must be met" to enable the test #1034
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
Comments
Tracked internally as rdar://136052793. |
I suspect we would probably implement this in terms of standard Boolean operators, not a bespoke DSL. |
@bkhouri @grynspan I like to take a lead on this issue. Before I start coding, I came up with a rough idea to ask for your opinion and then I will implement the rest of it. extension Trait where Self == ConditionTrait {
static func any(
_ traits: ConditionTrait...,
comment: Comment? = nil,
sourceLocation: SourceLocation = #_sourceLocation
) -> ConditionTrait {
Self(kind: .conditional {
try await traits._asyncContains { try await $0.evaluate() }
}, comments: Array(comment), sourceLocation: sourceLocation)
}
static func all(
_ traits: ConditionTrait...,
comment: Comment? = nil,
sourceLocation: SourceLocation = #_sourceLocation
) -> ConditionTrait {
Self(kind: .conditional {
try await traits._asyncAllSatisfy { try await $0.evaluate() }
}, comments: Array(comment), sourceLocation: sourceLocation)
}
}
extension Sequence where Element == ConditionTrait {
fileprivate func _asyncAllSatisfy(_ predicate: @escaping (Element) async throws -> Bool) async throws -> Bool {
for element in self {
if !(try await predicate(element)) {
return false
}
}
return true
}
fileprivate func _asyncContains(where predicate: @escaping (Element) async throws -> Bool) async throws -> Bool {
for element in self {
if try await predicate(element) {
return true
}
}
return false
}
}
@Test("Sample", .all(.any(.enabled, .enabled), .all(.enabled,.enabled)), .enabled)
func testSample() async throws {
...
} |
My thinking here was that we would implement Then you'd be able to write something like: @Test(.enabled(if: abc123) || .enabled(if: def456)) What makes this somewhat more complicated (regardless of how we spell the operations) is that To solve that, we may need to teach the operator implementations to treat |
@grynspan I completely agree that preserving correct information is essential. However, the example logic doesn’t seem accurate. According to De Morgan’s laws, the correct transformation should be: .disabled(if: x) || .disabled(if: y) == !(.enabled(if: x) && .enabled(if: y))
.enabled(if: !x) && .enabled(if: !y) == !(.disabled(if: x) || .disabled(if: y)) In abstract terms, the pseudo-code representation would look like this: extension Trait where Self == ConditionTrait {
static func &&(lhs: Self, rhs: Self) -> Self {
Self(kind: .conditional {
let l = try await lhs.evaluate()
let r = try await rhs.evaluate()
return l && r
})
}
static func ||(lhs: Self, rhs: Self) -> Self {
Self(kind: .conditional {
let l = try await lhs.evaluate()
let r = try await rhs.evaluate()
return l || r
})
}
static prefix func !(lhs: Self) -> Self {
Self(kind: .conditional {
let l = try await lhs.evaluate()
return !l
})
}
} That said, I recognize that this approach isn't optimized—it introduces unnecessary overhead without simplifying the logic. I'll work on a more efficient solution. |
That's not quite how these operators combine today if you just use them within a single trait, and I suspect that could be problematic. For instance: .disabled(if: x) || .disabled(if: y) Should probably be equivalent to: .disabled(if: x || y)
// i.e.
.enabled(if: !(x || y)) Not to: .enabled(if: !(x && y)) |
Correct me if I'm wrong, the expected end result should compose/merge logics instead of seeing like a boolean value? In other words (as you said), instead of If that’s not the case, the suggested logic interprets it as a boolean-like value (with ExpressibleByBooleanLiteral) and all nine fundamental laws for conjunction and disjunction applies to it. |
Right, my gut sense is that Does that make sense to you? |
Yes, it does, thanks for sharing that. It’s more of a grouping strategy rather than being a separation of logic. Like |
The implemented solution should consider readability of the conditions, in addition for "swift formatting". For example, how do we want to treat a "more complex" condition, say,
Granted, a "condition" like this might mean "the test is too complicated and needs to be broken down", but the "readability" aspect is still a thing. Whatever the solution, it would be great if we can an include each traits on it's own line as the following (which may be syntactically-incorrect) may be less 'readable" than the second e.g.:
And whatever the solution, it would be ideal to explicitly indicate every condition that was not met. |
From my perspective, the design should aim for simplicity while remaining flexible enough to support potential future grouping behaviors, such as I believe the core question in this issue is whether we should be grouping conditions or condition traits. Whatever approach we take, it should allow for merging, unions, and other relevant operations (for future). That being said, my goal isn't to overcomplicate the discussion. I’d like to propose a straightforward solution—something like Regarding your point, I see value in both suggestions, as each comes with its own trade-offs. For example, the term I'm currently working on a proposal and will share it here as soon as it's ready. Looking forward to hearing your thoughts! |
If you're planning to start a proposal in the near future, I will be on leave and probably won't be able to help much with it. @stmontgomery would you be able to advise @hrayatnia here? |
@stmontgomery sorry for long note, I tried to summarize and rule out as much as I can. Suggestion for Achieving
|
Option 1 | Option 2 |
---|---|
@Test("Name", .enabled(if: condition1 && condition2 && ...))
func testFoo() {} |
@Test("Name", .enabled(if: condition1), .enabled(if: condition2), ...)
func testFoo() {} |
Two-Layer Structure
Expand Code
Option 1 | Option 2 |
---|---|
@Test("Name", .enabled(if: condition1 && condition3), .enabled(if: condition2 && condition4), ...)
func testFoo() {} |
@Test("Name", .enabled(if: condition1 && condition2 && condition3 && condition4))
func testFoo() {} |
.any
Logic
Expand Code
@Test("Name", .enabled(if: condition1 || condition2 || condition3))
func testFoo() {}
This approach raises concerns regarding modularity, reusability, readability, and scalability, increasing the likelihood of errors. Additionally, tracking which condition caused an issue would be challenging.
Sometimes, ConditionTrait
s might have a correlation when used in @Test
, however when used as parameters, their meaning becomes ambiguous since, by default, there is no inherent relationship between parameters. As a result, the distinction between .all
and .any
can blur, as parameters at the same level hold equal priority (not for the compiler but for people like me) as a ConditionTraits
.
One possible resolution is to introduce a priority system (.HIGH
, .NORMAL
, .LOW
), which could help manage conditional logic. In this system, expressions might behave as .any
at a higher priority level and .all
at a same level. However, if expressions are independent but correlated, semantic errors may arise, leading to further complications in understanding the logic. in addition, prioritization is very limited and structured.
Proposal
Suggested Solution: Grouping ConditionTraits
(DSL)
Pros:
- Easy to read when the number of levels are low.
- Allows mixing logic as parameters.
- Supports more parameters, whether related or unrelated to conditions/traits.
- Separation of concerns.
- Resolves same-level ambiguity between
any
andall
.
Cons:
- Does not solve the root problem of tracking multiple subconditions. (out of the issue's scope)
- Introduces new methods beyond
disabled/enabled
, adding a learning curve. - Introduces Lisp-style.
- Misleading naming for DSL elements outside
enabled/disabled
could cause confusion.
Examples:
Original Code
@Test("Name", .any(.all(.disabled(if: condition1 && condition2),
.enabled(if: condition1 || condition3)),
.all(.disabled(if: condition4),
.disabled(method1))))
func testFoo() {}
Slightly Different Code
@Test("Name", .anyOf(.composeAll(.disabled(if: condition1 && condition2), // merge...
.enabled(if: condition1 || condition3)),
.composeAll(.disabled(if: condition4),
.disabled(method1))))
func testFoo() {}
or
@Test("Name", .any(of: .all(of: .disabled(if: condition1 && condition2), // merge...
.enabled(if: condition1 || condition3)),
.all(of: .disabled(if: condition4),
.disabled(method1))))
func testFoo() {}
Second Alternative Code (not my favorite)
@Test("Name", .grouped(.grouped(.disabled(if: condition1 && condition2),
.enabled(if: condition1 || condition3),
method: .allSatisfy),
.grouped(.disabled(if: condition4),
.disabled(method1)),
method: .allSatisfy),
method: .contains)
func testFoo() {}
Out of Scope:
New methods could be introduced easily; however, since the goal is to treat the expressions inside enabled
/disabled
as a composed behaviour, developers must always consider isDisabled
/isEnabled
, which can be annoying.
Suggested Solution: Grouping ConditionTraits
(Operators)
Pros:
- Avoids Lisp-style code.
- Compact and easy to read in simple cases.
- Highly reusable.
- it keeps changes to a minimum.
Cons:
- Does not solve the problem of tracking multiple subconditions. (out of the issue's scope)
- Custom operators might be preferable, it is rare to see standard operators having different meaning. (personal opinion)
- Reduces readability in complex cases unless well-structured.
- Introducing extra parameters and complex logic could be difficult.
- there's a small learning curve to it.
Example:
Custom Operator
@Test("Name", (.disabled(if: condition1 && condition2)
&&&
.enabled(if: condition1 || condition3))
|||
(.enabled(if: condition4) &&& .disabled(method1)))
func testFoo() {}
Suggested Solution: Introducing Conditions
and Grouping Conditions
(DSL)
Pros:
- Makes it easier to track which specific condition caused an issue. (out of the issue's scope)
- Simplifies grouping, separation, and organization of logical expressions.
- Enhances readability and helps maintain focus on logical expressions.
- Supports more parameters related to conditions.
- Separation of concerns.
- easier to introduce more methods such as
.none (!.all)
,.some(.filter{.tag,fileID,...})
,.optional()
,threshold(Compare count)
,etc.
Cons:
- Introduces computational overhead.
- Does not resolve same-layer confusion for multiple
enabled/disabled
conditions. (it could be resolved with prioritization) - Requires extra effort from developers to properly structure conditions and logic.
- A poorly implemented version could resemble Lisp-like syntax.
Examples:
first approach
@Test("Name", .disabled(.contains(.allSatisfy([condition1 && condition2, condition1 || condition3]),
.allSatisfy(condition4, method1))))
func testFoo() {}
Alternative
@Test("Name", .disabled([condition1 && condition2, condition1 || condition3].all,
[condition4, method1].all,
filter: .any))
func testBar() {}
Current System Implementation
@Test("Name", .disabled([condition1 && condition2,
condition1 || condition3].allSatisfy() ||
[condition4, method1].allSatisfy()))
func testFoo() {}
Suggestion
I believe a combination of custom operators on same-level ConditionTraits
and (3rd approach) additional methods on the Condition
collection could be a good trade-off. However, this is just my personal opinion, and I can prototype both approaches to provide insights on performance, ease of use, and potential issues.
Looking at that the below example (which is an actual usage here), I always have to look at the Swift Testing source code, or documentation, do determine if they are e.g.
Have we considered the how custom conditional traits can be used in the chosen solution? |
These are valid points, and I agree with the point regarding the implicit meaning of The only case can be problematic regardless of infix (custom operator) or prefix(DSL approach) situation is if P.S.: Those I wrote previously are merely suggestions; There’s still no solution unless you, @stmontgomery, and @grynspan as product owners say otherwise. |
In general, we wouldn't advise folks to create their own types that do what extension Trait where Self == ConditionTrait {
static func .skipInJenkins(sourceLocation: SourceLocation = #_sourceLocation) -> Self {
.disabled(if: inJenkins(), "Running in Jenkins", sourceLocation: sourceLocation)
}
} If there's some functionality that would require a completely custom trait type, I'm not sure what it is. |
As far as I know, we have no plans to make those symbols public, and |
Sorry for not being accurate in my previous comment. I didn’t mean that people would introduce new APIs to swift-testing. What I meant was that if swift-testing were to make However, since it seems it's not the case then no need to even entertain the hypotheticals. |
My original request, which may not have been clear, is to make usage of custom traits more explicit. That I, I want do define custom ConditionalTrais, and make their uses in the application of a trait explicit. Say I have the following defined in a module imported by a test
I want to be able to make this test more explicit with respect to the conditionals, without loosing the information/metadata the current implementaiton provideS, and where the solution indicate which conditions were [not] met causing the the test to be enabled/disabled.
|
@bkhouri That's already supported and functional though. |
But it's not explicit. I would like to have an equivalent of, where it's explicit, and where I can select e.g:
|
Would it be okay if I prototype all proposed solutions and prepare a range of test cases with condition traits—from simple to complex—for us to use as a basis for discussion? Regarding the DSL-style prefix approach (whether using extensions, enums, result builders, ...), I have some concerns about readability. In more complex scenarios, it tends to shift the developer's focus away from what really matters—the test logic itself—and toward the condition traits. (Granted SwiftUI is like that, but views are hierarchical, and focus should be on views.) Additionally, this approach tends to evolve into a deeply nested structure of conditions (akin to S-expressions or Lisp-style trees), introducing a learning curve for each new method or pattern added. Also, using names like Personally, I lean toward an infix-style solution using custom operators. It feels more idiomatic to Swift and helps guard against misuse by making intent and structure clearer at a glance. @bkhouri I would like to know your opinion on
I truly appreciate your time. Appendix: If it was in LISP(defun any (&rest conditions)
"Return T if any condition evaluates to true."
(some #'identity conditions))
(defun all (&rest conditions)
"Return T if all conditions evaluate to true."
(every #'identity conditions))
(defun conditionTraitsA () nil)
(defun conditionTraitsB () t)
(defun conditionTraitsC () nil)
(defun conditionTraitsD () t)
(defun conditionTraitsE () t)
(any
(any (all((conditionTraitsA) (conditionTraitsE) )) (conditionTraitsB) (conditionTraitsC))
(all (conditionTraitsD) any((conditionTraitsE) (conditionTraitsB)))
;; => T |
It is a non-goal of the testing library to add multiple ways to accomplish the exact same task. Both AND and OR can already be expressed, trivially, with combinations of What we are missing right now is a way to express AND/OR operations on Any changes here should be designed with that specific goal in mind. I don't think the Testing Workgroup would approve a proposal adding |
There are a lot of ideas being explored here, several which would result in a pretty sophisticated but also potentially complicated API. I'd like to come back to an earlier idea suggested above, about overloading the standard As a general guideline, we have tried to make Swift Testing's APIs easy to learn and understand by even a casual user, and to do that we have leaned on existing Swift APIs, syntax, and concepts wherever possible. I would say we have a strong preference towards using standard Swift language features instead of using custom DSLs. The trait system and built-in traits are probably the area where we adhere to that principle the least strongly, but still, whenever there is an opportunity to use existing things, I think we should try hard to go that route. |
@stmontgomery @grynspan I agree with both of your comments completely, and I'm inclined to infix operator solution ( a) shuffled off into a helper function P.S.: Regarding the P.S. 2: here I only addressed what is the current system, why it's needed, and what possible solutions are. |
Personally, I just want the readability to be explicit on the application of the conditional traits. Using the
How would it look using the @hrayatnia : I'd be happy to see code example of different solutions on the application. |
@bkhouri that would be like: @Test(
.skipSwiftCISelfHosted(
"These packages don't use the latest runtime library, which doesn't work with self-hosted builds."
) &&
.requireUnrestrictedNetworkAccess("Test requires access to https://github.com/") &&
.skipHostOS(.windows, "Issue #8409 - random.swift:34:8: error: unsupported platform")
)
func testExamplePackageDealer() throws { } or @Test(
.skipSwiftCISelfHosted(
"These packages don't use the latest runtime library, which doesn't work with self-hosted builds."
) ||
.requireUnrestrictedNetworkAccess("Test requires access to https://github.com/") ||
.skipHostOS(.windows, "Issue #8409 - random.swift:34:8: error: unsupported platform")
)
func testExamplePackageDealer() throws { } Right now, I'm working on the behind-the-scenes logic for subconditions and other parts. As soon as that one is finished, I will provide more examples; however, meanwhile, there are some examples of all suggested cases here (sorry, I have the habit of making code collapsible when text is too large; in example segments you find some examples). |
Updates the `GroupedConditionTrait` to prepare for tests concurrently, improving performance. Also adds a check to ensure there are at least two condition traits before attempting to operate on them, preventing potential errors.
Addresses a scenario where a grouped condition trait might contain only a single condition. This change ensures that the single condition is properly handled, by evaluating or preparing it.
Sorry for the long absence. I got time to work on this issue again over Easter. First Solution modified files:
I came up with a few solutions which are neither optimized nor formatted yet. I like to continue with the first solution and clean it up (both in terms of memory usage and code style). However, before I continue, I would like to know your opinions. |
Motivation
As a test case author, I want to explicitly enable, or disable, a test when:
true
true
I always prefer explicit vs implicit. Take the following example
It is unclear by ready the code whether the various conditions are
||
(OR-ed), or whether they are&&
(AND-ed). A developer must be aware of Swift Testing behaviour to know when the test will be enabled or disabled.Proposed solution
One option is to introduce a
.any
and.all
traits, that would accept a list of traits, or any number of traits.Consider the following, which is more explicit. In the first,
conditionA && conditionB
must be true to enable the test, while theconditionA || conditionB
must be true to enable the second testThe
.any
and.all
become especially useful when custom condition traits are added, where using the single.enabled(if: condition)
is more challenging.Alternatives considered
I'm open to other solution
Additional information
No response
The text was updated successfully, but these errors were encountered: