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

Support throwing errors from within @Test attributes #166

Closed
jmjauer opened this issue Jan 1, 2024 · 4 comments · Fixed by #366
Closed

Support throwing errors from within @Test attributes #166

jmjauer opened this issue Jan 1, 2024 · 4 comments · Fixed by #366
Labels
enhancement New feature or request public-api Affects public API

Comments

@jmjauer
Copy link

jmjauer commented Jan 1, 2024

Description

The argument parameter of Test(_:_:arguments:) is currently very limited.

Adding a version of Test(_:_:arguments: () async throws -> C) would greatly improve the parameterisation possibilities (e.g. async loading of parameters from a file).

Expected behavior

No response

Actual behavior

No response

Steps to reproduce

No response

swift-testing version/commit hash

No response

Swift & OS version (output of swift --version && uname -a)

No response

@jmjauer jmjauer added the enhancement New feature or request label Jan 1, 2024
@grynspan
Copy link
Contributor

grynspan commented Jan 2, 2024

The @Test macro supports async arguments. It doesn't currently support throwing arguments because that's somewhat harder to model in a reasonable way and we need to figure out what it means to throw an error from within an attribute.

Does that help at all?

@jmjauer
Copy link
Author

jmjauer commented Jan 2, 2024

The @Test macro supports async arguments.

I didn't know that - great!

It doesn't currently support throwing arguments because that's somewhat harder to model in a reasonable way and we need to figure out what it means to throw an error from within an attribute.

Wouldn't it be easy to allow () async throws -> C as an argument and let the test fail if an error is thrown? The advantage of a closure would be that a test could be more self-contained, without helper functions providing the arguments. Allowing a throwing closure would also reduce the boilerplate required by wrapping throwing code in do catch blocks and letting the error fail (or crash). So instead of:

func myArguments() async -> [String] {
    do {
        try await ...
    } catch {
        ...
    }
}

@Test("test all strings", arguments: await myArguments())
func allStrings(stirng: String) {
    ...
}

we could write

@Test("test all strings") {
    try await ...
}
func allStrings(stirng: String) {
    ...
}

I have not spent any time with macros in Swift, so there may be a limitation I am not aware of.

@grynspan
Copy link
Contributor

grynspan commented Jan 2, 2024

It is not hard to allow the developer to express try in that position—what's hard is figuring out how to actually handle the error if it occurs, and there are multiple answers that seem reasonable.

A failure to generate an arguments collection for a test means we cannot fully instantiate that test as specified by the developer. We've been leaning toward saying that we should instead instantiate a monomorphic Test instance that fails immediately with the thrown error, however this results in a test with a physical structure that does not match what the developer intended, and that could be confusing. We already do something similar for tests with unavailable arguments (i.e. @available prevented them from being resolved) but such tests are skipped at runtime anyway, so it has less of a visible impact on the test output.

we could write

@test("test all strings") {
try await ...
}
func allStrings(stirng: String) {
...
}

This would not be grammatically correct Swift, so you'd have to write something like @Test("test all strings", arguments: { try await ... }) instead. Supporting a closure in that position would require additional overloads to the Test macro. While this is technically feasible, I think we'd need some strong justification for adding those overloads when your initial example (minus the do/catch, as we are planning to support try here in the future) is equivalent.

@grynspan grynspan changed the title Add advanced parameterisation to Test(_:_:arguments:) Support throwing errors from within @Test attributes Jan 2, 2024
@jmjauer
Copy link
Author

jmjauer commented Jan 2, 2024

Ok, that makes sense. Adding try would be a great addition to the api.

Thanks for your time!

@grynspan grynspan added the public-api Affects public API label Feb 26, 2024
stmontgomery added a commit that referenced this issue Apr 19, 2024
… run, and support throwing expressions

Resolves #166
Resolves rdar://121531170
stmontgomery added a commit that referenced this issue Apr 19, 2024
… run, and support throwing expressions

Resolves #166
Resolves rdar://121531170
stmontgomery added a commit that referenced this issue Apr 24, 2024
… run, and support throwing expressions

Resolves #166
Resolves rdar://121531170
stmontgomery added a commit that referenced this issue Apr 29, 2024
… run, and support throwing expressions (#366)

This changes test declaration, discovery, planning, and running such
that arguments to parameterized test functions are evaluated lazily,
only after determining their test will run. It also adds support for
throwing expressions in test arguments.

### Motivation:

When defining parameterized test functions, it can be useful to call
throwing or asynchronous APIs — for example, to fetch the arguments from
an external source. Today, the `@Test` macro supports `async`
expressions but not `throws`. More problematic still: the arguments of
_all_ tests get evaluated, even for tests which don't actually run
(which may happen for various reasons). Evaluating the arguments of a
test which won't run wastes time and resources, so we should try to
avoid this, and support `throws` expressions too for better flexibility.

### Modifications:

- Modify the `@Test` macro and supporting library interfaces to surround
test arguments in closures, so their evaluation can be lazy and
deferred.
- Modify how test case arguments are stored on each `Test` instance
accordingly.
- Modify the planning logic in `Runner.Plan` to defer evaluation of test
cases until we determine whether each test will run.
- Update tests.

### Result:

Now, arguments to a parameterized test will only be evaluated if that
test is actually going to run, and the expressions may include `try`
and potentially throw an `Error`.

### Checklist:

- [x] Code and documentation should follow the style of the [Style
Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md).
- [x] If public symbols are renamed or modified, DocC references should
be updated.

Resolves #166
Resolves rdar://121531170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request public-api Affects public API
Projects
None yet
2 participants