-
Notifications
You must be signed in to change notification settings - Fork 67
SWIFT-400 Improve Evergreen test result output #533
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
Conversation
| AUTH=${AUTH} \ | ||
| SWIFT_VERSION=${SWIFT_VERSION} \ | ||
| sh ${PROJECT_DIRECTORY}/.evergreen/run-tests.sh | ||
| ${PROJECT_DIRECTORY}/.evergreen/run-tests.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipefail isn't supported by dash, which was being used instead of bash here on Ubuntu
| let failureCount: Int | ||
|
|
||
| /// Converts this test suite to XML. | ||
| func toXML() -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered actually using an XML encoder as there are a few Codable-style ones floating around, but that would require making this a library rather than a standalone script. since the XML is so simple it didn't seem worth it.
| mutating func processLine(_ line: String) throws { | ||
| let fullRange = NSRange(line.startIndex..<line.endIndex, in: line) | ||
|
|
||
| if let match = Self.testCaseStartedRegex.firstMatch(in: line, range: fullRange) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to store a map of regex: handler function here so this could be done with a for loop rather than separate statements, but the compiler did not like my attempting to store mutating instance methods in a dictionary. I was running into the error described here, which I had not seen before but is a rather interesting case.
| let escapedFailure = failure | ||
| .replacingOccurrences(of: "\"", with: """) | ||
| .replacingOccurrences(of: "'", with: "'") | ||
| .replacingOccurrences(of: "<", with: "﹤") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL there are "small" versions of many unicode symbols, lol. the "right" way to do this in XML is to use e.g. " instead of " but that doesn't end up being very readable in the HTML logs.
Codecov Report
@@ Coverage Diff @@
## master #533 +/- ##
=======================================
Coverage 77.09% 77.09%
=======================================
Files 132 132
Lines 14095 14095
=======================================
Hits 10866 10866
Misses 3229 3229 Continue to review full report at Codecov.
|
patrickfreed
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I have one question on some formatting but otherwise LGTM (don't need to review again). I really like the state-machine approach to parsing the tests.
etc/convert-test-results.swift
Outdated
| case none(completeSuites: [TestSuite]) | ||
|
|
||
| // account for variation in formatting of test output on platforms. this assumes you are running this script on the | ||
| // same platform where you ran the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is super frustrating and I think one of those warts exposed by the ObjC implementation on macOS. I remember looking into this a while back for some reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I think this is actually why xcpretty doesn't work on Linux, because it only checks for the macOS format - that might be why this came up for you
| mutating func processLine(_ line: String) throws { | ||
| let fullRange = NSRange(line.startIndex..<line.endIndex, in: line) | ||
|
|
||
| if let match = Self.testCaseStartedRegex.firstMatch(in: line, range: fullRange) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the spacing of this if / else chain is pretty spread out--was that done on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nah, I think I originally had some comments or something that I deleted, removed. this actually made me realize I forgot to lint the file so I did that and made a few small fixes
Putting this up as I'm calling it finished, but no pressure to review it during Skunkworks.
This adds a script written in Swift which reads input (which should be
swift testoutput) from stdin, parses it into data structures, converts those structures to XML, and then writes the resulting XML to stdout.This script is now used when running our tests on Evergreen to upload the results.
With a small modification (updating list of suite names to skip) this should be reusable for the BSON repo.