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

Duplicates occurring when generating unrolled causes test failures #1098 #1099

Conversation

pbielicki
Copy link

@pbielicki pbielicki commented Feb 21, 2020

This change is Reviewable

@codecov
Copy link

codecov bot commented Feb 21, 2020

Codecov Report

Merging #1099 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1099      +/-   ##
============================================
+ Coverage     74.12%   74.13%   +0.01%     
- Complexity     3472     3476       +4     
============================================
  Files           385      385              
  Lines         10678    10683       +5     
  Branches       1309     1309              
============================================
+ Hits           7915     7920       +5     
  Misses         2302     2302              
  Partials        461      461
Impacted Files Coverage Δ Complexity Δ
.../runtime/extension/builtin/UnrollNameProvider.java 100% <100%> (ø) 15 <1> (+2) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 628cfb5...4e9b2bb. Read the comment docs.

Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

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

How about appending the iteration count in brackets instead (like SafeIterationNameProvider does) in case a duplicate name is detected? That way the feature wouldn't have to be opt-in.

@Vampire
Copy link
Member

Vampire commented Feb 22, 2020

I wouldn't enforce that, a user might like it that iterations are named the same. For example the navigation in IJ then works properly.

@pbielicki
Copy link
Author

@marcphilipp that is another valid option, but I'd like this to be a conscious decision of the user, thus hidden behind a (possibly yet another) flag

@marcphilipp
Copy link
Member

For example the navigation in IJ then works properly.

Does it when using a pattern?

@Vampire
Copy link
Member

Vampire commented Feb 23, 2020

When using a pattern other than #featureName, the iterations are not named the same most probably. But if that is the pattern or there is actually no pattern used, the iterations are named all like the feature and navigation works. And with my fancy naming PR, you could rely on the the default naming to have the data variables in the name and if you want to have better IDE navigation temporarily, you can set a system variable to the pattern of #featureName and all iterations are named like the feature.

@leonard84
Copy link
Member

What are we trying to solve here?

  1. You don't notice duplicated names, but you probably don't have a problem. Not ideal, but you probably won't go searching for the flag for a problem you don't even know about.
  2. If you already noticed non-unique names, then you could just fix them.

So if we hide it behind a flag, it would probably only be used by you. However, if we don't do that, then we add the performance penalty to everyone. And I bet that at best only very few people are affected by this issue.

This could also be done with as a third-party extension, so instead of setting the flag you'd add the extension. I'm personally leaning to this solution.

At the end of the day, it is probably an issue with the reporting tool, at least for Spock 2.x, since it will report a UniqueId in addition to the displayed name.

@pbielicki pbielicki closed this May 25, 2021
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.

Duplicates occurring when generating unrolled test names should cause test failures
4 participants