-
Notifications
You must be signed in to change notification settings - Fork 50
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
Fix missing test failures #40
base: master
Are you sure you want to change the base?
Conversation
A shame that this was never addressed and with @joshdholtz bringing trainer into fastlane the bug exists in fastlane. |
@tahirmt I'll be bringing these PRs over! Only so much I can do 🙃 |
@joshdholtz I made some fixes to the junit xml format on my fork that I was using. Didn't create a pull request here because the repo seemed abandoned. And also because I don't know enough about fastlane/ruby to be able to create lane context outputs and created environment variable outputs for test results which isn't how it should've been. |
@tahirmt I'd be happy to work with you to get your fork merged into fastlane if you would like to create a PR there! I'm sure there are others that would like the features that you see that are needed. |
@joshdholtz in my fork I added this pull request as is and fixed the junit output format because the message wasn't quite right. These were all the changes. tahirmt#1 |
Better late than never I suppose 😅 |
@robnadin My sincerest apologies! So many different areas to maintain but I really should have given trainer more time 😔 It should be easier for me know that it's in fastlane and I have some more experience with it not. I really appreciate your the time and effort on this PR. |
@robnadin would it be possible for you to create the PR on https://github.com/fastlane/fastlane? I lack a lot of context and ruby knowledge to be able to implement this there and having the file and line references in junit file make a big difference in the quality of the report. We are holding off on updating fastlane because of this. |
* Fix xcpretty_naming for xcresult * Add test result object hierarchy * Refactored group and name parsing function
de2662d
to
7a84a65
Compare
@joshdholtz finally managed to create the PR incorporating the changes from this PR as well as the |
These fixes came about due to the way the test result format has changed between Xcode 10 and 11, and how the logic to parse
xcresult
bundles in Trainer was not finding any of our failing tests.In our use case, we were finding that test failures were not being parsed correctly because we were grouping our tests from a collection of other test suites like so:
This results in
find_failure
incorrectly comparingAllTests/testFoo
toTestsA/testFoo
and returning false due to theidentifier
andsanitized_test_case_name
values not matching in the test metadata.Instead we should call
xcresulttool get --format json
and pass it theid
we get from the summary reference in the metadata to get an accurate representation of the test failure summary. The benefit of this approach is that we also get explicit file name and line number information that we are able to format in exactly the same way as Trainer has been doing with the oldertest_result
bundles in Xcode 10 and earlier.