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

Add ConditionTrait.evaluate() #909

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Uncommon
Copy link

Add ConditionTrait.evaluate() so that a condition can be evaluated independent of a Test object.

Motivation:

Currently, the only way a ConditionTrait is evaluated is inside the prepare(for:) method. This makes it difficult and awkward for third-party libraries to utilize these traits because evaluating a condition would require creating a dummy Test to pass to that method.

Modifications:

Add ConditionTrait.evaluate(), and ConditionTrait.Evaluation enum for the return value.

Result:

Public API allows for evaluating a ConditionTrait in any context.

Checklist:

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

@Uncommon
Copy link
Author

I'm trying to decide what tests to add. Are existing tests enough, since I pretty much just moved existing logic to a different place?

@grynspan
Copy link
Contributor

We aim for 100% code coverage when testing, so the new function should be tested (even if it is just a trivial "did this return true when I expected it to?" test.)

@grynspan grynspan added enhancement New feature or request public-api Affects public API api-proposal API proposal PRs (documentation only) labels Jan 14, 2025
@grynspan grynspan added this to the Swift 6.x milestone Jan 14, 2025
var commentOverride: Comment?


/// Returns the result of evaluating the condition.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Returns the result of evaluating the condition.
/// Evaluate this instance's underlying condition.
///
/// - Returns: The result of evaluating this instance's underlying condition.
///

You may also want to include a discussion paragraph after - Returns: if there is additional information for the reader to consume. In particular, it may be worth noting that the result of this function is not cached.

@Uncommon
Copy link
Author

I plan to add some tests soon, at which point I think it will be appropriate to take the PR out of draft mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-proposal API proposal PRs (documentation only) enhancement New feature or request public-api Affects public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants