test: cleanup text-field unit tests covered by mixins tests #8715
Conversation
| }); | ||
| }); | ||
|
|
||
| describe('binding', () => { |
There was a problem hiding this comment.
All tests in this suite are covered here:
| }); | ||
| }); | ||
|
|
||
| describe('has-value attribute', () => { |
There was a problem hiding this comment.
Moved tests for number and boolean to this suite, others are already covered:
| }); | ||
| }); | ||
|
|
||
| describe(`methods`, () => { |
There was a problem hiding this comment.
Both tests in this suite are covered here:
| await assertAttrCanBeSet(prop, true); | ||
| await assertAttrCanBeSet(prop, false); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('clear button', () => { |
There was a problem hiding this comment.
Moved the default value test to this suite, all the rest are already covered there:
| @@ -159,74 +99,5 @@ describe('text-field', () => { | |||
| expect(textField.hasAttribute('focused')).to.be.true; | |||
| }); | |||
| }); | |||
|
|
|||
| describe('autoselect', () => { | |||
There was a problem hiding this comment.
Moved two tests to this suite:
web-padawan
force-pushed
the
test/restructure-text-field-tests
branch
from
8c3ee54 to
7750b0e
Compare
| fire(input, 'input'); | ||
| expect(textField.value).to.be.equal('foo'); | ||
| it('should set ariaTarget property to the input element', () => { | ||
| expect(textField.ariaTarget).to.equal(textField.inputElement); |
There was a problem hiding this comment.
The ariaTarget property was not covered at all (tests passed with it being removed).
I also added explicit tests for focusElement and stateTarget for consistency.
|
| ['disabled'].forEach((prop) => { | ||
| it(`should set boolean property ${prop}`, async () => { |
There was a problem hiding this comment.
Covered here:
| }); | ||
| }); | ||
|
|
||
| ['autocomplete', 'autocorrect', 'readonly', 'required'].forEach((prop) => { |
There was a problem hiding this comment.
Covered here:
| ['autocomplete'].forEach((prop) => { | ||
| it(`should set boolean attribute ${prop}`, async () => { | ||
| await assertAttrCanBeSet(prop, 'on'); | ||
| }); | ||
| }); | ||
|
|
||
| ['autocapitalize'].forEach((prop) => { |
There was a problem hiding this comment.
Covered here:
|
This ticket/PR has been released with Vaadin 24.7.0.alpha12 and is also targeting the upcoming stable 24.7.0 version. |
Description
The following unit tests are now covered by mixins, and we have
typingstests that ensure the text field does apply them:autoselect- covered byInputControlMixinclearButtonVisible- covered byClearButtonMixinbinding(value) - covered byInputMixinclear()method - covered byInputMixinAlso the property delegation logic was moved to mixins:
disabledproperty - covered byDelegateFocusMixinautocorrect,autocomplete,autocapitalize- covered byInputFieldMixinplaceholder,readonly,required,title- covered byInputControlMixinIMO having these tests only in
@vaadin/text-fieldpackage is not necessary. In particular, they are not present in other field components that use same mixins e.g.@vaadin/number-field- in V14, this component was based on text-field but it's no longer the case.Note: since we don't have JS API to test that the component uses certain mixins (see #2357), we have to rely on the typings tests, which is not ideal. However, this would only break the component if some mixins are changed in
.jsbut not in.d.tswhich would be an issue on its own. So IMO we can rely on typings tests for now.Type of change