Skip to content

Conversation

@kmahar
Copy link
Contributor

@kmahar kmahar commented Oct 28, 2020

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 test output) 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.

@kmahar kmahar requested a review from patrickfreed October 28, 2020 23:18
AUTH=${AUTH} \
SWIFT_VERSION=${SWIFT_VERSION} \
sh ${PROJECT_DIRECTORY}/.evergreen/run-tests.sh
${PROJECT_DIRECTORY}/.evergreen/run-tests.sh
Copy link
Contributor Author

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 {
Copy link
Contributor Author

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) {
Copy link
Contributor Author

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: "")
Copy link
Contributor Author

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. &quot; instead of " but that doesn't end up being very readable in the HTML logs.

@codecov-io
Copy link

codecov-io commented Oct 29, 2020

Codecov Report

Merging #533 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

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

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1953dcc...bb4cdf1. Read the comment docs.

Copy link
Contributor

@patrickfreed patrickfreed left a 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.

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.
Copy link
Contributor

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.

Copy link
Contributor Author

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) {
Copy link
Contributor

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?

Copy link
Contributor Author

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

@kmahar kmahar merged commit 5b98b0f into master Nov 5, 2020
@kmahar kmahar deleted the parse-test-results branch November 5, 2020 23:27
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.

4 participants