-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Improve failing log output of tests #840
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
fab2ef9
Invoke `XCTFail()` on each corrections
norio-nomura 1beed4b
Produce detailed log of generated violations from nonTriggeringExamples
norio-nomura 9436e67
Produce detailed log when triggeringExample without expected location…
norio-nomura ae14c90
Produce detailed log when triggeringExample violated at unexpected lo…
norio-nomura 76fd4b5
Produce detailed log when expected location of triggeringExample is n…
norio-nomura 5237110
Remove surplus assertions from triggeringExamples
norio-nomura 6ba6be2
Avoid line_length violation
norio-nomura cc24409
Apply reviews
norio-nomura File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,38 @@ private func cleanedContentsAndMarkerOffsets(from contents: String) -> (String, | |
return (contents as String, markerOffsets.sorted()) | ||
} | ||
|
||
private func render(violations: [StyleViolation], in contents: String) -> String { | ||
var contents = (contents as NSString).lines().map { $0.content } | ||
for violation in violations.sorted(by: { $0.location > $1.location }) { | ||
guard let line = violation.location.line, | ||
let character = violation.location.character else { continue } | ||
|
||
let message = String(repeating: " ", count: character - 1) + "^ " + [ | ||
"\(violation.severity.rawValue.lowercased()): ", | ||
"\(violation.ruleDescription.name) Violation: ", | ||
violation.reason, | ||
" (\(violation.ruleDescription.identifier))"].joined() | ||
if line >= contents.count { | ||
contents.append(message) | ||
} else { | ||
contents.insert(message, at: line) | ||
} | ||
} | ||
return (["```"] + contents + ["```"]).joined(separator: "\n") | ||
} | ||
|
||
private func render(locations: [Location], in contents: String) -> String { | ||
var contents = (contents as NSString).lines().map { $0.content } | ||
for location in locations.sorted(by: > ) { | ||
guard let line = location.line, let character = location.character else { continue } | ||
var content = contents[line - 1] | ||
let index = content.index(content.startIndex, offsetBy: character - 1) | ||
content.insert("↓", at: index) | ||
contents[line - 1] = content | ||
} | ||
return (["```"] + contents + ["```"]).joined(separator: "\n") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this equivalent and simpler? return "```\n\(contents)\n```" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here also |
||
} | ||
|
||
extension Configuration { | ||
fileprivate func assertCorrection(_ before: String, expected: String) { | ||
guard let path = NSURL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true) | ||
|
@@ -105,32 +137,51 @@ extension XCTestCase { | |
let nonTriggers = ruleDescription.nonTriggeringExamples | ||
|
||
// Non-triggering examples don't violate | ||
XCTAssertEqual(nonTriggers.flatMap({ violations($0, config: config) }), []) | ||
for nonTrigger in nonTriggers { | ||
let unexpectedViolations = violations(nonTrigger, config: config) | ||
if unexpectedViolations.isEmpty { continue } | ||
let nonTriggerWithViolations = render(violations: unexpectedViolations, in: nonTrigger) | ||
XCTFail("nonTriggeringExample violated: \n\(nonTriggerWithViolations)") | ||
} | ||
|
||
var violationsCount = 0 | ||
var expectedViolationsCount = 0 | ||
// Triggering examples violate | ||
for trigger in triggers { | ||
let triggerViolations = violations(trigger, config: config).sorted { | ||
$0.location < $1.location | ||
} | ||
violationsCount += triggerViolations.count | ||
let triggerViolations = violations(trigger, config: config) | ||
|
||
// Triggering examples with violation markers violate at the marker's location | ||
let (cleanTrigger, markerOffsets) = cleanedContentsAndMarkerOffsets(from: trigger) | ||
if markerOffsets.isEmpty { | ||
expectedViolationsCount += 1 | ||
if triggerViolations.isEmpty { | ||
XCTFail("triggeringExample did not violate: \n```\n\(trigger)\n```") | ||
} | ||
continue | ||
} | ||
expectedViolationsCount += markerOffsets.count | ||
let file = File(contents: cleanTrigger) | ||
let expectedLocations = markerOffsets.map { Location(file: file, characterOffset: $0) } | ||
|
||
// Assert violations on unexpected location | ||
let violationsAtUnexpectedLocation = triggerViolations | ||
.filter { !expectedLocations.contains($0.location) } | ||
if !violationsAtUnexpectedLocation.isEmpty { | ||
XCTFail("triggeringExample violate at unexpected location: \n" + | ||
"\(render(violations: violationsAtUnexpectedLocation, in: trigger))") | ||
} | ||
|
||
// Assert locations missing vaiolation | ||
let violatedLocations = triggerViolations.map { $0.location } | ||
let locationsWithoutViolation = expectedLocations | ||
.filter { !violatedLocations.contains($0) } | ||
if !locationsWithoutViolation.isEmpty { | ||
XCTFail("triggeringExample did not violate at expected location: \n" + | ||
"\(render(locations: locationsWithoutViolation, in: cleanTrigger))") | ||
} | ||
|
||
XCTAssertEqual(triggerViolations.count, expectedLocations.count) | ||
for (triggerViolation, expectedLocation) in zip(triggerViolations, expectedLocations) { | ||
XCTAssertEqual(triggerViolation.location, expectedLocation, | ||
"'\(trigger)' violation didn't match expected location.") | ||
} | ||
} | ||
// Triggering examples violate | ||
XCTAssertEqual(violationsCount, expectedViolationsCount) | ||
|
||
// Comment doesn't violate | ||
XCTAssertEqual( | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Isn't this equivalent and simpler?
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.
contents
is[String]
.