Fix CI failing tests #464
Fix CI failing tests #464
Conversation
lib/Validator.js
Outdated
|
|
||
| // Here slotType value tested and is ok! | ||
| continue; |
There was a problem hiding this comment.
With continue here, the anyOf, allOf, etc. processing code further down inside this for block was not being reached.
lib/Validator.js
Outdated
| // If every condition gives format error, just return one error | ||
| // TODO does this apply in allOf, second branch of exactlyOneOf? | ||
| if (results.every((result) => result === parse_error)) { | ||
| return parse_error; | ||
| } | ||
| return results.join('\n'); |
There was a problem hiding this comment.
Test:
expect(fn('hello')).toEqual('Value does not match format for xsd:integer')
We were getting 'Value does not match format for xsd:integer\Value does not match format for xsd:integer' without this new if condition.
Was the test wrong? Do want duplicated format error messages across two anyOf conditions? If so, I would remove this if block and edit the test.
Or do we want to keep this if block, and add a similar one for allOf, and the second return branch of exactlyOneOf?
There was a problem hiding this comment.
Keep two error messages, but let's make them more descriptive. Which anyOf conditions are they violating?
| const slotName = fieldSymbolsAtIndex[index].slotName; | ||
| let slotTitle = fieldSymbolsAtIndex[index].slotTitle; | ||
| // If slotTitle not specified; use slotName as slotTitle | ||
| slotTitle = slotTitle === undefined ? slotName : slotTitle; | ||
|
|
||
| tempObject[slotTitle] = slotName; |
There was a problem hiding this comment.
Is this what we want? A test was failing previously because the fields global var in fields.test.js was missing slotTitle values. There are several lines of code downstream of this function that expect slotTitle values. I just re-used slotName as title as well to fix things.
| // We do not process any nested anyOf, allOf, etc. | ||
| return parse_error |
There was a problem hiding this comment.
a_big_or_small_number: {
name: 'a_big_or_small_number',
range: 'integer',
any_of: [{ maximum_value: 10 }, { minimum_value: 100 }],
}
'hello' will return a parse error === 'Value does not match format for xsd:integer'
The test suite expects us to return this error immediately, so we must return parse error here. Otherwise we will try to process the any_of unnecessarily.
Previously, there was a continue further down that I think was meant to stop such unnecessary processing of any_of, all_of, etc. But I had to comment it out because it was also stopping processing in situations without an initial parse error--where we actually want to process any_of, all_of, etc. e.g., '11' should return 'Value is greater than maximum value\nValue is less than minimum value'
yarn.lock
Outdated
lib/Toolbar.js
Outdated
There was a problem hiding this comment.
A bunch of this stuff was done automatically by Prettier. I renamed a variable from className to _className
Edit: I should pull the latest changes from master before removing wip label.