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

Fix missing test failures #40

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

Conversation

robnadin
Copy link

  • Fix xcpretty_naming for xcresult
  • Add test result object hierarchy
  • Refactored group and name parsing function

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:

class AllTests: XCTestCase {
    
    override class var testCaseClasses: [XCTestCase.Type] {
        return [
            TestsA.self,
            TestsB.self,
            TestsC.self,
        ]
    }
    
    override class var defaultTestSuite: XCTestSuite {
        let testSuite = XCTestSuite(forTestCaseClass: self)
        let testCases = testCaseClasses.flatMap { testCaseClass in
            testCaseClass.testInvocations.map { testCaseClass.init(invocation: $0) }
        }
        testCases.forEach(testSuite.addTest)
        return testSuite
    }
}

...

class TestsA: XCTestCase {
    
    func testFoo() {
        XCTAssertTrue(false)
    }
}

This results in find_failure incorrectly comparing AllTests/testFoo to TestsA/testFoo and returning false due to the identifier and sanitized_test_case_name values not matching in the test metadata.

Instead we should call xcresulttool get --format json and pass it the id 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 older test_result bundles in Xcode 10 and earlier.

@tahirmt
Copy link

tahirmt commented Jan 26, 2022

A shame that this was never addressed and with @joshdholtz bringing trainer into fastlane the bug exists in fastlane.

@joshdholtz
Copy link
Member

joshdholtz commented Jan 26, 2022

@tahirmt I'll be bringing these PRs over! Only so much I can do 🙃

@tahirmt
Copy link

tahirmt commented Jan 26, 2022

@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.

@joshdholtz
Copy link
Member

@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.

@tahirmt
Copy link

tahirmt commented Jan 26, 2022

@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

@robnadin
Copy link
Author

Better late than never I suppose 😅

@joshdholtz
Copy link
Member

@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.

@tahirmt
Copy link

tahirmt commented Feb 14, 2022

@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
@tahirmt
Copy link

tahirmt commented Mar 23, 2022

@joshdholtz finally managed to create the PR incorporating the changes from this PR as well as the junit format changes from my fork. fastlane/fastlane#20105.

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.

3 participants