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

Are _all_ failures for a given test reported? #52

Open
mokagio opened this issue Mar 2, 2023 · 2 comments
Open

Are _all_ failures for a given test reported? #52

mokagio opened this issue Mar 2, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@mokagio
Copy link
Contributor

mokagio commented Mar 2, 2023

🌎 Environment

  • collector-swift version: 0.3.0
  • Xcode: 14.2 (14C18)
  • Swift: Apple Swift version 5.7.2 (swiftlang-5.7.2.135.5 clang-1400.0.29.51)
  • OS: macOS 13.0.1 (22A400)

💬 Description

Our test report shows three failed assertions for a test, but the Test Analytics page only shows one.

🦶 Reproduction Steps

I noticed the issue in WordPress iOS, so I suppose one could run the tests and hope to run into a flaky one... 🤔 But that sounds like a lot of work to verify this behavior!

Another option could be to create an ad hoc project, write an async test that has multiple assertions, and see how its results are reported.

🤔 Expected Results

I'm not actually sure what to expect here to be honest. I guess what stood out to me is the inconsistency between the test result from the .xcresult file and the Test Analytics page, see screenshot below.

The good news is that one of the failures is reported, so there's no coarse grained information lost: The fact that the test failed is tracked. 👍

There is a wider questions of whether or not tests should have more than one assertion. Regardless of what one feels on the matter, it would be uncomfortable for a library to force a certain style of testing on its users. For what is forth, I try to take a pragmatic approach. There are many instances in which multiple assertions make the test code more compact and readable.

📸 Screenshots

Notice the orange annotation. There are three failures in the xcresult for testDeleteFollowerFailure but only one in Test Analytics.

image

(Apologies for the arrow noise in the screenshot. I annotated it to service two issue in one go, the other one being #51.)

The failures were:

image

I'm not sure what sorting strategy if any is used to report the failures in our Buildkite annotation, but it's worth noting that the first failure reported is the one that appears on Test Analytics.

📎 Attachments

N.A.

🤝 Related

@mokagio mokagio added the bug Something isn't working label Mar 2, 2023
@iampatbrown
Copy link
Collaborator

@mokagio Hey mate! Thanks for this :)

I'm not actually sure what to expect here to be honest.

This is probably something that needs backend consideration. I'm pretty sure sending multiple failures individually right now would increase the test run count.

The good news is that one of the failures is reported, so there's no coarse grained information lost: The fact that the test failed is tracked.

Actually all failures are being sent as a single test failure with multiple failure reasons. I've included a small change in #44 that should make this a bit clearer but it's not a long term solution. I imagine the failure reasons could be counted and displayed on the dashboard at the moment though.

There is a wider questions of whether or not tests should have more than one assertion. Regardless of what one feels on the matter, it would be uncomfortable for a library to force a certain style of testing on its users.

I think more than one assertion is fine. And I don't think this library would enforce any particular style. It's probably more about the dashboard presenting the data in a way that's suitable for each platform.

@blaknite might have a better idea of what's planned for and potentially if something could be done in the dashboard to help with this one.

@mokagio
Copy link
Contributor Author

mokagio commented Mar 21, 2023

Actually all failures are being sent as a single test failure with multiple failure reasons. I've included a small change in #44 that should make this a bit clearer but it's not a long term solution. I imagine the failure reasons could be counted and displayed on the dashboard at the moment though.

Good to hear 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants