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

Some Examples were not recognized #343

Merged
merged 3 commits into from
May 11, 2016

Conversation

magicmonty
Copy link

Examples are not recognized on some test frameworks if the examples are too long

return c.Resolve<MarkdownProvider>();
}
}).SingleInstance();
builder.Register(
Copy link
Member

Choose a reason for hiding this comment

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

A request for the future: try not to give in to resharper's temptation to optimize code that is unrelated to the feature you're working on. I know it's difficult :-)

I won't reject the PR because of this but please keep it in mind for the future.

@magicmonty
Copy link
Author

Ok, sorry for that

@dirkrombauts dirkrombauts merged commit 1e51b8e into picklesdoc:develop May 11, 2016
@dirkrombauts
Copy link
Member

Thanks again for your hard work over the last few days!

@dirkrombauts
Copy link
Member

Released as version 2.6.2

@magicmonty magicmonty deleted the LongExamples branch May 11, 2016 18:17
@magicmonty
Copy link
Author

No problem, I use this currently in production, and I'm happy to help if I find something. It's also a fun side project

var name = SpecFlowNameMapping.Build(scenarioOutline.Name.ToLowerInvariant());
stringBuilder.Append(name).Append("\\(");

foreach (var value in row.Select(v => v.Length > MaxExampleValueLength ? new { Value = v.Substring(0, MaxExampleValueLength), Ellipsis = "..." } : new { Value = v, Ellipsis = "" }))

Choose a reason for hiding this comment

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

@magicmonty and @dirkrombauts ... can you provide a bit more explanation why we need to limit the "signature" for searching examples in NUnit3 to 37 characters? This has kept some of the tests in our environment from showing Pass / Fail on the generated Pickles site. If I simply remove the logic to trim these signatures and allow for the full length, it works fine.

Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid I don't remember why it had to be 37 characters ... @magicmonty do you remember?

Copy link
Author

Choose a reason for hiding this comment

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

If I remember correctly, then the output in the result XML is was truncated by 37 characters (at least at the time of writing) (see https://github.com/picklesdoc/pickles/blob/develop/src/Pickles/Pickles.TestFrameworks.UnitTests/NUnit/NUnit3/results-example-nunit3.xml#L574)

Copy link
Member

Choose a reason for hiding this comment

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

@magicmonty - thanks!

@ocsurfnut - you say if you remove the logic to trim, it works fine. Do you mean that it works on your feature files, or do you mean that the unit tests in Pickles pass?

Choose a reason for hiding this comment

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

@dirkrombauts I didn't run the unit tests (yet). It "works fine" in that the generated Pickles site has the test results correlated correctly for these examples. Otherwise, with the trimming, the scenario shows as Inconclusive.

Copy link
Member

Choose a reason for hiding this comment

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

NUnit 3.6 does no longer truncate long results. @ocsurfnut thanks for bringing this to my attention. I will publish a new version soon.

Choose a reason for hiding this comment

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

Thank you @dirkrombauts , pls tag me on your revision when you get around to it. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

@ocsurfnut I released a fix for the Nunit 3 problem in version 2.13.0.

Choose a reason for hiding this comment

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

@dirkrombauts , thank you for the patch! I've implemented 2.13.0 on our pipeline and that resolved the issue.

Copy link
Member

Choose a reason for hiding this comment

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

@ocsurfnut glad to hear it!

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