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 Expect.oneOf #126

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sashaafm
Copy link

This PR adds the Expect.oneOf function. This function works similarly to Expect.all, but results in a pass if at least one of the subjects passes the check. If all subjects fail the check it returns a fail.

I think the implementation should be okay (I had implemented the same function in my codebase, but without the usage of the internal types). I'm not sure about the tests because I'm having a hard time running the repository tests locally.

Any feedback would be appreciated!

[] ->
Test.Expectation.fail
{ reason = Invalid EmptyList
, description = "Expect.oneOf ran out of expectations."
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the feedback should be a lot more informative. This doesn't tell you at all how to fix your test.

Copy link
Author

@sashaafm sashaafm Sep 6, 2023

Choose a reason for hiding this comment

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

@gampleman could you please help provide a solution for that? Would it need to keep track of why all expectations failed and print that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although perhaps in a more condensed format? I think there's some experimentation to be done on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants