Migrate to new spec #106
Migrate to new spec #106
Conversation
* set_default_environment can be a file path or a struct with the
environment
* equality for struct elements were using Ion equality instead of PTS
equality
* Interpolation now accepts both $variable or ${variable} when
interpolating strings
* using absolute path for test files in errors for better error
reporting
* JUnitRunner generated name now includes the test location location along with
the test id
Migration was done mostly manually to keep the formatting as it's important for tests to be readable. A lot of tests align parameters which would be impossible to maintain if we used ion-java to transform
All pts tests are run through junit in the regular build
| @@ -37,18 +37,20 @@ successfully or produce an error: | |||
| Changes the default environment for the current file. The initial default environment is the empty environment. | |||
|
|
|||
| ```ion | |||
| set_default_environment::<struct, required - new default environment> | |||
| set_default_environment::<struct|string, required - new default environment> | |||
There was a problem hiding this comment.
I would use Path instead of string and then below this line add
where Path is a string and denotes the relative path to ...
There was a problem hiding this comment.
I'm keeping string instead of path since I'm listing the valid Ion types that can go there. I reworded the sentence on line 43 to explain what the string represents
testscript/Readme.md
Outdated
| A `for` function contains a `template` and `variableSet`. Each `test` in the template is performed once for | ||
| each `variableSet` by substituting a `variableSet`'s name/value pairs into corresponding placeholders in a `test`. | ||
| A `for` function contains a `template` and `variable_set`. Each `test` in the template is performed once for | ||
| each `variable_set` by substituting a `variable_set`'s name/value pairs into corresponding placeholders in a `test`. |
There was a problem hiding this comment.
I might be reading this wrong, placeholders are in the template right?
There was a problem hiding this comment.
yes, in each templated test. Will change to just template since "templated test" is too pedantic
| var lastOffset = 0 | ||
| private fun IonSequence.foldInterpolating(target: IonSequence, variables: IonStruct) = | ||
| this.fold(target) { acc, el -> | ||
| val interpolated = el.interpolate(variables) |
There was a problem hiding this comment.
minor: why the extra local val just inline acc.add(el.interpolate(variables)) . I assume add is mutating so you cannot apply the inline one more time and get a one-liner.
There was a problem hiding this comment.
the extra val was to make debugging easier, forgot to change it back. Will make it a one liner
| statement: "char_length('${in}')", | ||
| expected: $result | ||
| }, | ||
| /* |
There was a problem hiding this comment.
no it's a different test. Commented it out to fix some issues and forgot to uncomment...
| ], | ||
|
|
||
| variable_sets: [ | ||
| { in: "", result: (success 0 ) }, |
There was a problem hiding this comment.
nit: in Sexpressions close paren does not need an extra space, i.e., (a ). Instead write (a)
Migrated tests from old spec to PTS spec
Also fixes a multiple PTS bugs and integrates it with the ref implementation
* set_default_environment can be a file path or a struct with the
environment
* equality for struct elements were using Ion equality instead of PTS
equality
* Interpolation now accepts both $variable or ${variable} when
interpolating strings
* using absolute path for test files in errors for better error
reporting
* JUnitRunner generated name now includes the test location location along with
the test id
Migrates existing integ tests to the new spec and integrate them with the ref implementation junit test suite.
PR is fairly large and it's split into 3 commits to make it a little easier to review:
Original tests are still part of the build, will be removed as part of a separate PR
closes #89