Apply iteration name provides to simple features as well#1238
Apply iteration name provides to simple features as well#1238leonard84 merged 3 commits intospockframework:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1238 +/- ##
============================================
+ Coverage 76.56% 76.57% +0.01%
- Complexity 3748 3753 +5
============================================
Files 405 405
Lines 11418 11424 +6
Branches 1389 1389
============================================
+ Hits 8742 8748 +6
Misses 2196 2196
Partials 480 480
Continue to review full report at Codecov.
|
Vampire
left a comment
Vampire
left a comment
There was a problem hiding this comment.
I'm not sure whether this is good and especially consistent.
The old way was also kind of a hack and it is questionable how and if it should be supported and if, how to do it consistently.
I extended your test example a bit, added @Unroll to parameterized and added a parameterized2 with no annotation for 1.3 and @Rollup for 2.0.
So we have:
test => simple feature
parameterized => unrolled feature (without the extension)
parameterized2 => uprolled feature (without the extension)
With 1.3 I get this:
With 2.0 master I get this:
With this PR I get this:
So in 1.3
- it works on simple features name
- it does not work on already unrolled DD-iterations (DD = data-driven)
- it "works" on uprolled DD-iterations, unrolling them and defining the name, not appending
In 2.0 master
- it does not work on simple features name
- it does not work on DD-features name
- it works on already unrolled DD-iterations, appending
- it "works" on uprolled DD-iterations, unrolling them and defining the name, not appending
In this PR
- it does work on simple features name
- it does not work on DD-features name
- it works on already unrolled DD-iterations, appending
- it "works" on uprolled DD-iterations, unrolling them and defining the name, not appending
The original issue essentially was about difference between Spock 1 and 2, but with this PR it is still differente between the two and it is also inconsistent a bit in itself, for example it now works for the feature name of a simple feature but not for the feature name of a DD-feature.
|
|
||
| @Retention(RetentionPolicy.RUNTIME) | ||
| @Target([ElementType.TYPE]) | ||
| @Inherited |
There was a problem hiding this comment.
This is just superfluous here, isn't it?
There was a problem hiding this comment.
yes it was c&p from the example code.
|
Is it really inconsistent? The sample extension explicitly sets |
|
The unrolled / uprolled is not so much the inconsistency but more showing that this is maybe more a hack than a proper feature. |
|
Well the simple feature also represents a single iteration, whereas the parametrized feature does not have its own iteration, as it is only a container for the individual iterations. |
|
That's about technical details and then it depends on the exact terminology used. But I'm talking less about the technical correctness but more about the perceived correctness as seen by the user. |
|
My usage of this in Spock 1 was a “hack” insofar as it was abusing this mechanism in order to augment test names with global parameterisation context. We have a system where we parameterise entire test suites via system properties when launching, and desire to have the parameters be apparent in the reported test names. It's a very important aspect of our Spock usage. Without understanding all of the details, I can see that a mechanism more focussed on augmenting the “reported test name” would be better than relying on this indirect mechanism. I don't think it would be a problem if that mechanism is different between 1 and 2. |
|
@ldaley take another look ( |
to separate name and displayName. The new displayName is now used to set the reported name. The name field should now always contain the raw value although existing extensions might still modify it. fixes spockframework#1236 Undo
|
@leonard84 LGTM. Looking forward to using it. |
4 participants
fixes #1236