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

Improve failing log output of tests #840

Merged
merged 8 commits into from
Dec 2, 2016
Merged

Conversation

norio-nomura
Copy link
Collaborator

On debugging failing tests, I did adhoc improvements sometime. But now, I think it should be implemented that properly.

This depends on #832

  • Assert on each correction of testSwiftLintAutoCorrects()
  • Produce detailed log of generated violations from nonTriggeringExamples :

/Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:136: error: -[SwiftLintFrameworkTests.RulesTests testMissingDocs] : failed - nonTriggeringExample violated:

/// docs
public func a() {}
       ^ warning: Missing Docs Violation: Public declarations should be documented. (missing_docs)
  • Produce detailed log when triggeringExample without expected location does not violate:

/Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:159: error: -[SwiftLintFrameworkTests.RulesTests testValidDocs] : failed - triggeringExample did not violate:

/// docs
public func no() -> (Int, Int) {return (1, 2)}
  • Produce detailed log when triggeringExample violated at unexpected location:

/Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:173: error: -[SwiftLintFrameworkTests.RulesTests testMissingDocs] : failed - triggeringExample violate at unexpected location:

public func a↓() {}
       ^ warning: Missing Docs Violation: Public declarations should be documented. (missing_docs)
  • Produce detailed log when expected location of triggeringExample is not violated at:

/Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:180: error: -[SwiftLintFrameworkTests.RulesTests testMissingDocs] : failed - triggeringExample did not violate at expected location:

public func a↓() {}
  • Remove surplus assertions from triggeringExamples:
    • Comparing total count of violations and expected locations from all triggering examples
    • Comparing count of violations and expected locations on each triggering examples
    • Order based comparison between violations and expected locations

@jpsim
Copy link
Collaborator

jpsim commented Nov 25, 2016

This is unfortunately quite out of date due to depending on #832, and my lack of reviewing that.

@norio-nomura these seem like great improvements, maybe you could update this branch once #832 lands?

e.g.
> /Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:136: error: -[SwiftLintFrameworkTests.RulesTests testMissingDocs] : failed - nonTriggeringExample violated:
> ```
> /// docs
> public func a() {}
>        ^ warning: Missing Docs Violation: Public declarations should be documented. (missing_docs)
> ```
… does not violate

e.g.
> /Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:159: error: -[SwiftLintFrameworkTests.RulesTests testValidDocs] : failed - triggeringExample did not violate:
> ```
> /// docs
> public func no() -> (Int, Int) {return (1, 2)}
> ```
…cation

e.g.
> /Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:173: error: -[SwiftLintFrameworkTests.RulesTests testMissingDocs] : failed - triggeringExample violate at unexpected location:
> ```
> public func a↓() {}
>        ^ warning: Missing Docs Violation: Public declarations should be documented. (missing_docs)
> ```
…ot violated at.

e.g.
> /Users/norio/github/SwiftLint/Tests/SwiftLintFramework/TestHelpers.swift:180: error: -[SwiftLintFrameworkTests.RulesTests testMissingDocs] : failed - triggeringExample did not violate at expected location:
> ```
> public func a↓() {}
> ```
Removed:
- Comparing total count of violations and expected locations from all triggering examples
- Comparing count of violations and expected locations on each triggering examples
- Order based comparison between violations and expected locations
@norio-nomura norio-nomura force-pushed the nn-improve-failing-log-of-tests branch from 1efb572 to 6ba6be2 Compare December 1, 2016 00:24
@norio-nomura
Copy link
Collaborator Author

Rebased.

Copy link
Collaborator

@jpsim jpsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 other than a few minor suggestions

@@ -34,6 +34,38 @@ private func cleanedContentsAndMarkerOffsets(from contents: String) -> (String,
return (contents as String, markerOffsets.sorted())
}

func render(violations: [StyleViolation], in contents: String) -> String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private?

return (["```"] + contents + ["```"]).joined(separator: "\n")
}

func render(locations: [Location], in contents: String) -> String {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private?

contents.insert(message, at: line)
}
}
return (["```"] + contents + ["```"]).joined(separator: "\n")
Copy link
Collaborator

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?

return "```\n\(contents)\n```"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contents is [String].

content.insert("↓", at: index)
contents[line - 1] = content
}
return (["```"] + contents + ["```"]).joined(separator: "\n")
Copy link
Collaborator

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?

return "```\n\(contents)\n```"

Copy link
Collaborator Author

@norio-nomura norio-nomura Dec 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also contents is [String]

@jpsim jpsim changed the base branch from swift3.0 to master December 2, 2016 05:54
@jpsim jpsim merged commit 811ab1d into master Dec 2, 2016
@jpsim jpsim deleted the nn-improve-failing-log-of-tests branch December 2, 2016 05:55
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.

2 participants