trainer icon indicating copy to clipboard operation
trainer copied to clipboard

Fix missing test failures

Open robnadin opened this issue 4 years ago • 9 comments

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

robnadin avatar Mar 18 '20 11:03 robnadin

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

tahirmt avatar Jan 26 '22 14:01 tahirmt

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

joshdholtz avatar Jan 26 '22 14:01 joshdholtz

@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 avatar Jan 26 '22 15:01 tahirmt

@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 avatar Jan 26 '22 15:01 joshdholtz

@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. https://github.com/tahirmt/trainer/pull/1

tahirmt avatar Jan 26 '22 15:01 tahirmt

Better late than never I suppose 😅

robnadin avatar Jan 27 '22 09:01 robnadin

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

joshdholtz avatar Jan 27 '22 13:01 joshdholtz

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

tahirmt avatar Feb 14 '22 17:02 tahirmt

@joshdholtz finally managed to create the PR incorporating the changes from this PR as well as the junit format changes from my fork. https://github.com/fastlane/fastlane/pull/20105.

tahirmt avatar Mar 23 '22 04:03 tahirmt