[Rest Api Compatibility] transformations for keys in match and length#72156
[Rest Api Compatibility] transformations for keys in match and length#72156joegallo merged 16 commits intoelastic:masterfrom
Conversation
Additional transformations to modify a key in a match and a key in a length assertions
|
Pinging @elastic/es-delivery (Team:Delivery) |
| */ | ||
| public class ReplaceMatch implements RestTestTransformByParentObject { | ||
| private final String replaceKey; | ||
| private final String newKeyName; |
There was a problem hiding this comment.
This needs to be exposed as an @Input. Adding a public getter for this field would also remove the need to override it in ReplaceKeyInMatch.
...src/main/java/org/elasticsearch/gradle/internal/rest/compat/RestCompatTestTransformTask.java
Outdated
Show resolved
Hide resolved
...st/java/org/elasticsearch/gradle/internal/test/rest/transform/length/ReplaceLengthTests.java
Outdated
Show resolved
Hide resolved
jakelandis
left a comment
There was a problem hiding this comment.
A few suggestions on class organization.
...src/main/java/org/elasticsearch/gradle/internal/rest/compat/RestCompatTestTransformTask.java
Outdated
Show resolved
Hide resolved
| Collections.singletonList(new ReplaceLength("key.in_length_to_replace", "key.in_length_replaced", null)) | ||
| ); | ||
|
|
||
| AssertObjectNodes.areEqual(transformedTests, expectedTransformation); |
There was a problem hiding this comment.
This sooooo much more readable ! Thanks for addressing this.
| this(replaceKey, replaceKey, replacementNode, testName); | ||
| } | ||
|
|
||
| public ReplaceMatch(String replaceKey, String newKeyName, JsonNode replacementNode, String testName) { |
There was a problem hiding this comment.
Can you change the name of this class (and associated paramenters) to an abstract class ReplaceByKey with getKeyToFind() as an abstract method and then reintroduce ReplaceMatch as a child of this class that defines
@Override
@Internal
public String getKeyToFind() {
return "match";
}
This would mean that ReplaceLength (which should be ReplaceKeyInLength) and ReplaceKeyInMatch would extend ReplaceByKey .
So...
RestTestTransformByParentObject -> ReplaceByKey -> [ ReplaceMatch, ReplaceKeyInMatch, ReplaceKeyInLength]
jakelandis
left a comment
There was a problem hiding this comment.
Changes look good! (one minor, non blocking nitpick request)
Additional yml tests transformations to modify a key in a match assertion and a key in a
length assertion.
The testing is done with the original and expected transformed files. This should make it a little more easy to write new tests and add new transformations.