Update formfiller to support elements with type datetime #238
Conversation
|
Hey @freddieptf Would you have a few minutes to review this? |
|
@freddieptf Ping |
|
|
||
| it('#234 - fill datetime expected input format', () => { | ||
| return expect(harness.fillForm('bug_234', ['2023-01-01'])) | ||
| .to.be.rejectedWith('expect input in format'); |
There was a problem hiding this comment.
This error message is just for the test and not what is actually thrown, yes?
There was a problem hiding this comment.
This is the actual error throw. The assertion here confirms the substring is present.
test/app-forms.spec.js
Outdated
| expect(result.errors).to.be.empty; | ||
| }); | ||
|
|
||
| it('#234 - fill datetime expected input format', () => { |
There was a problem hiding this comment.
Maybe this can be a bit more descriptive, like fill datetime expected correct input format so intent is clear at a glance
There was a problem hiding this comment.
good idea. i've updated the names for all tests
| }); | ||
|
|
||
| it('#234 - datetime field is required', async () => { | ||
| const result = await harness.fillForm('bug_234', ['2023-01-01 x']); |
There was a problem hiding this comment.
Why not fail with the input format error?
There was a problem hiding this comment.
Currently the form filler is pretty dumb: it just checks that there is a delimited input and it puts the first value in the first box + the second input in the second box.
I think this is probably a good design decision for a test tool. Ideally, test tools would be able to test whatever inputs a human can provide. If the form filler only filled vaild well-formed inputs in the proper format, then it is actually a less useful test tool.
This test is failing because the input is "fillable" but it is failing enketo's validation. I've updated the test name to reflect that.
There was a problem hiding this comment.
oh i see, thanks for clearing that up
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
#234
cc @benkags