[#442] account for lists in YamlUtils#loadYaml
#443
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also fixes a bug with the `hasDouble`, `hasInt`, and `hasBoolean` methods of `YamlObject`. Adds tests for `YamlUtils` including one to catch the root list issue. Finally, also updates the POM migration to account for POMs without dependcy management sections.
ewilkins-csi
commented
Oct 28, 2024
| @@ -78,23 +97,23 @@ public String getString(String... path) { | |||
| } | |||
|
|
|||
| public boolean hasInt(String... path) { | |||
| return hasValue(int.class, path); | |||
| return hasValue(Integer.class, path); | |||
There was a problem hiding this comment.
I: these were returning false when they shouldn't have as the map has the boxed version of the value so isInstance was failing.
ewilkins-csi
commented
Oct 28, 2024
| @@ -73,7 +73,8 @@ | |||
| "implementation": "com.boozallen.aissemble.upgrade.migration.v1_10_0.DeltaSparkYamlMigration", | |||
| "fileSets": [ | |||
| { | |||
| "includes": ["**/*.yaml"] | |||
| "includes": ["**/*.yaml"], | |||
| "excludes": ["**/target/**", "**/templates/**"] | |||
There was a problem hiding this comment.
I: Not strictly necessary but this cuts down significantly on the number of output lines talking about un-parseable helm templates.
| public void theDecimalValueOfThePropertyIs(String name, double value) { | ||
| String type = yaml.get(name).getClass().getSimpleName(); | ||
| assertTrue("Property was not loaded as double: " + name + " (was " + type + ")", yaml.hasDouble(name)); | ||
| assertEquals("Property value does not match for: " + name, value, yaml.getDouble(name), 0.01); |
There was a problem hiding this comment.
It's a delta param that they added to floating point assertions. Avoids the tricky floating point rounding issues where you get 0.000000000001 or some other minuscule difference just from how FP is represented. So this says treat the numbers as equal if they are within 0.01 of each other.
csun-cpointe
approved these changes
Oct 28, 2024
carter-cundiff
approved these changes
Oct 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Also fixes a bug with the
hasDouble,hasInt, andhasBooleanmethods ofYamlObject. Adds tests forYamlUtilsincluding one to catch the root list issue. Finally, also updates the POM migration to account for POMs without dependency management sections.