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

Fail test if the expected and actual traces differ (#777) #781

Merged
merged 3 commits into from
Apr 24, 2018

Conversation

burmanm
Copy link
Contributor

@burmanm burmanm commented Apr 23, 2018

Use pretty.Diff(expected, actual) instead of reflect.DeepEqual(expected, actual) to check for equality between given and received trace. Also, if there is a difference, set the test to fail instead of only logging the failed trace.

And prettify the storage-integration-test output

@coveralls
Copy link

coveralls commented Apr 23, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 3e42d37 on burmanm:fix_storage_integration_test into 210fc0b on jaegertracing:master.

@ghost ghost assigned yurishkuro Apr 23, 2018
@ghost ghost added the review label Apr 23, 2018
yurishkuro
yurishkuro previously approved these changes Apr 23, 2018
}

diff := pretty.Diff(expected, actual)
if len(diff) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

nit: combine into one line: if diff := pretty.Diff(expected, actual); len(diff) > 0 {


diff := pretty.Diff(expected, actual)
if len(diff) > 0 {
t.Logf("Expected and actual differ: %v\n", diff)
Copy link
Member

Choose a reason for hiding this comment

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

when there are many differences, this doesn't print them in easily readable way. I prefer the loop as before.

@yurishkuro yurishkuro dismissed their stale review April 23, 2018 18:49

a couple nits

@burmanm burmanm force-pushed the fix_storage_integration_test branch from 6cac6a7 to 008eec8 Compare April 24, 2018 06:45
Use pretty.Diff(expected, actual) instead of reflect.DeepEqual(expected, actual) to check for equality between given and received trace. Also, if there is a difference, set the test to fail instead of only logging the failed trace.

Signed-off-by: Michael Burman <miburman@redhat.com>
@burmanm burmanm force-pushed the fix_storage_integration_test branch from 008eec8 to 455a85d Compare April 24, 2018 06:51
@burmanm
Copy link
Contributor Author

burmanm commented Apr 24, 2018

Yep, modified based on the review.

@yurishkuro
Copy link
Member

looks like the builds are failing due to #786


if diff := pretty.Diff(expected, actual); len(diff) > 0 {
for _, d := range diff {
t.Logf("Expected and actual differ: %v\n", d)
Copy link
Member

Choose a reason for hiding this comment

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

nit: \n is not necessary, but ok

@yurishkuro yurishkuro merged commit 2325502 into jaegertracing:master Apr 24, 2018
@ghost ghost removed the review label Apr 24, 2018
@yurishkuro
Copy link
Member

thanks @burmanm !

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.

3 participants