From 8b0f4394536169d10cd005e331608afde678faa6 Mon Sep 17 00:00:00 2001 From: Jerry Chen Date: Tue, 12 Aug 2025 16:10:36 -0700 Subject: [PATCH] New constructor for envvar-based condition trait creation The new condition trait constructor makes it easy to skip/unskip tests based on the *presence* of a specific environment variable. It does not attempt to inspect the value to determine if it is "truthy" or not. Need to resolve the issue of thread safety with the envvar unit tests added here before merging. Some alternative options/extensions to consider: * **Allow specifying multiple environment variables?** Could use a variadic or something here to allow that, although maybe that is an exceedingly rare case. The disadvantage would be that it would be a little ambiguous if you need to match on "all" or "any" env variable with this approach. The current implementation has the advantage of being quite simple and probably meeting the majority of use cases. * **Check for truthy/falsy values for matching env vars?** For example, 0/false is considered "falsy", everything else considered "truthy". Then enabled means the env var is present with a truthy value, and disabled means the env var is missing or present with a falsy value. The main disadvantage here is that this logic would be more complicated and perhaps more surprising to users of the condition trait. --- Sources/Testing/Traits/ConditionTrait.swift | 52 +++++++++++++++++++ .../Traits/ConditionTraitTests.swift | 45 ++++++++++++++-- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/Sources/Testing/Traits/ConditionTrait.swift b/Sources/Testing/Traits/ConditionTrait.swift index d60cf7cce..290908666 100644 --- a/Sources/Testing/Traits/ConditionTrait.swift +++ b/Sources/Testing/Traits/ConditionTrait.swift @@ -149,6 +149,32 @@ extension Trait where Self == ConditionTrait { Self(kind: .conditional(condition), comments: Array(comment), sourceLocation: sourceLocation) } + /// Constructs a condition trait that disables a test if an environment + /// variable is not present. + /// + /// - Parameters: + /// - comment: An optional comment that describes this trait. + /// - sourceLocation: The source location of the trait. + /// - environmentName: The name of an environment variable. If it is found + /// the current environment, the trait allows the test to run. + /// Otherwise, the testing library skips the test. The value of + /// the environment variable is not inspected. + /// + /// - Returns: An instance of ``ConditionTrait`` that checks for the provided + /// environment variable. + @_spi(Experimental) + public static func enabled( + ifEnvironmentPresent environmentName: String, + _ comment: Comment? = nil, + sourceLocation: SourceLocation = #_sourceLocation + ) -> Self { + Self( + kind: .conditional { + Environment.variable(named: environmentName) != nil + }, comments: Array(comment), sourceLocation: sourceLocation + ) + } + /// Constructs a condition trait that disables a test unconditionally. /// /// - Parameters: @@ -207,4 +233,30 @@ extension Trait where Self == ConditionTrait { ) -> Self { Self(kind: .conditional { !(try await condition()) }, comments: Array(comment), sourceLocation: sourceLocation) } + + /// Constructs a condition trait that disables a test if an environment + /// variable is present. + /// + /// - Parameters: + /// - comment: An optional comment that describes this trait. + /// - sourceLocation: The source location of the trait. + /// - environmentName: The name of an environment variable. If it is not + /// found the current environment, the trait allows the test to run. + /// Otherwise, the testing library skips the test. The value of + /// the environment variable is not inspected. + /// + /// - Returns: An instance of ``ConditionTrait`` that checks for the provided + /// environment variable. + @_spi(Experimental) + public static func disabled( + ifEnvironmentPresent environmentName: String, + _ comment: Comment? = nil, + sourceLocation: SourceLocation = #_sourceLocation + ) -> Self { + Self( + kind: .conditional { + Environment.variable(named: environmentName) == nil + }, comments: Array(comment), sourceLocation: sourceLocation + ) + } } diff --git a/Tests/TestingTests/Traits/ConditionTraitTests.swift b/Tests/TestingTests/Traits/ConditionTraitTests.swift index 9747f6b3f..009ae0170 100644 --- a/Tests/TestingTests/Traits/ConditionTraitTests.swift +++ b/Tests/TestingTests/Traits/ConditionTraitTests.swift @@ -8,7 +8,7 @@ // See https://swift.org/CONTRIBUTORS.txt for Swift project authors // -@testable import Testing +@_spi(Experimental) @testable import Testing @Suite("Condition Trait Tests", .tags(.traitRelated)) struct ConditionTraitTests { @@ -37,7 +37,19 @@ struct ConditionTraitTests { .disabled(if: false) ) func disabledTraitIf() throws {} - + + @Test( + ".enabled if a certain env var exists", + .enabled(ifEnvironmentPresent: "TEST_ENV_VAR") + ) + func enabledEnvironmentPresentIf() throws {} + + @Test( + ".disabled if a certain env var exists", + .disabled(ifEnvironmentPresent: "TEST_ENV_VAR") + ) + func disabledEnvironmentPresentIf() throws {} + @Test func evaluateCondition() async throws { let trueUnconditional = ConditionTrait(kind: .unconditional(true), comments: [], sourceLocation: #_sourceLocation) @@ -45,7 +57,7 @@ struct ConditionTraitTests { let enabledTrue = ConditionTrait.enabled(if: true) let enabledFalse = ConditionTrait.enabled(if: false) var result: Bool - + result = try await trueUnconditional.evaluate() #expect(result) result = try await falseUnconditional.evaluate() @@ -55,4 +67,31 @@ struct ConditionTraitTests { result = try await enabledFalse.evaluate() #expect(!result) } + + // TODO: What do we wanna do about envvar thread safety? This won't be safe to + // run in parallel alongside any other env var based tests because it ends up + // calling setenv. + @Test + func evaluateConditionEnvironmentVariable() async throws { + let enabledEnvironment = ConditionTrait.enabled(ifEnvironmentPresent: "TEST_ENV_VAR") + let disabledEnvironment = ConditionTrait.disabled(ifEnvironmentPresent: "TEST_ENV_VAR") + var result: Bool + + result = try await enabledEnvironment.evaluate() + #expect(!result) + result = try await disabledEnvironment.evaluate() + #expect(result) + + try #require(Environment.setVariable("1", named: "TEST_ENV_VAR")) + result = try await enabledEnvironment.evaluate() + #expect(result) + result = try await disabledEnvironment.evaluate() + #expect(!result) + + try #require(Environment.setVariable("0", named: "TEST_ENV_VAR")) + result = try await enabledEnvironment.evaluate() + #expect(result, "Actual value of the environment variable shouldn't matter") + result = try await disabledEnvironment.evaluate() + #expect(!result, "Actual value of the environment variable shouldn't matter") + } }