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

teamcity reporter: output location of test failure #626

Merged
merged 1 commit into from
Nov 28, 2019

Conversation

dlipovetsky
Copy link
Contributor

Fixes #623

@dlipovetsky dlipovetsky force-pushed the teamcity-reporter-failure-location branch from 4eb47f3 to 331039d Compare November 15, 2019 23:17
@blgm
Copy link
Collaborator

blgm commented Nov 18, 2019

Thank you @dlipovetsky. I don't have much context on the teamcity reporter, and it has only been changed once in the past few years - by @manosnoam. So I'll leave this a few days in case @manosnoam has any feedback, since sometimes changes to formats can sometimes break other teams.

@manosnoam
Copy link
Contributor

manosnoam commented Nov 19, 2019

Hi @dlipovetsky,
What's the difference between failure.ComponentCodeLocation.String() and failure.Location.String() ?

Can you also give an output (or screenshot) example from TeamCity ?

@dlipovetsky
Copy link
Contributor Author

dlipovetsky commented Nov 19, 2019

Hi @manosnoam!

Thanks for reviewing, and for implementing the TeamCity reporter 😄

Hi @dlipovetsky,
What's the difference between failure.ComponentCodeLocation.String() and failure.Location.String() ?

ComponentCodeLocation is the line of code that defines the Ginkgo "Component" (Ginko's internal name for what its docs refer to as blocks and specs, i.e., Describe/Context/It/BeforeSuite, etc). See here for some details; unfortunately, there aren't any comments.

Location is the line of code where the test failed, e..g, the line where an assertion failed. When a test fails, Location is an important piece of information. Without it, you need to search where precisely the test failed.

For comparison, the JUnit reporter outputs both locations when a test fails.

Can you also give an output (or screenshot) example from TeamCity ?

I'll try to get one that doesn't leak any internal information 😄 .

@dlipovetsky
Copy link
Contributor Author

So I'll leave this a few days in case @manosnoam has any feedback, since sometimes changes to formats can sometimes break other teams.

Thanks for the review @blgm! You make a good point.

The TeamCity message contains two attributes, message and details. This PR adds a single line of information to details.

For example:

Before:

[testFailed name="some test" message=''/path/to/test.go:100 details="unlucky"]

After

[testFailed name="some test" message=''/path/to/test.go:100 details="unlucky|n/path/to/test.go:20"]

I only expect TeamCity itself to parse the service messages. I don't believe TeamCity parses the contents of details at all. While TeamCity collects test statistics based on the test name; the message and details are not relevant.

…finition

Previously, only the test definition was output. When a test fails, the
location of the failure is important to know, especially for complex tests.

Signed-off-by: Daniel Lipovetsky <dlipovetsky@d2iq.com>
@dlipovetsky dlipovetsky force-pushed the teamcity-reporter-failure-location branch from 331039d to aaadeac Compare November 20, 2019 00:08
@dlipovetsky
Copy link
Contributor Author

Rebased.

@dlipovetsky
Copy link
Contributor Author

@manosnoam Your thoughts?

@manosnoam
Copy link
Contributor

Looks good to me.

A side note:
The location of test failure in code is very helpful, but it is not always clear in Ginkgo output/stacktrace.

For example, it has happened to me that an assertion failed inside an It block:

Ω(output.TestCases[0].ClassName).Should(Equal("My test suite"))

But then the stacktrace did not refer to that exact line of assertion, but only to the first line of the block:

It("should record the test as having failed", func() {

In this case it is unknown what assertion actually broke the test...
This of-course should be opened as a different issue to fix.

@blgm
Copy link
Collaborator

blgm commented Nov 28, 2019

@dlipovetsky thank you for this contribution!

@manosnoam thank you for reviewing!

@blgm blgm merged commit 9869142 into onsi:master Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Teamcity Reporter does not include location of failure
3 participants