Skip to content
This repository has been archived by the owner on May 26, 2019. It is now read-only.

Fix second test in autocomplete-component #2271

Merged
merged 1 commit into from
Mar 14, 2018
Merged

Fix second test in autocomplete-component #2271

merged 1 commit into from
Mar 14, 2018

Conversation

algo74
Copy link
Contributor

@algo74 algo74 commented Mar 11, 2018

At least in my case, the test was falling because triggerKeyEvent(this.element.querySelector('.list-filter input'), "keyup", 83); apparently did not modify the value of the input field.

At least in my case, the test was falling because `triggerKeyEvent(this.element.querySelector('.list-filter input'), "keyup", 83);` apparently did not modify the value of the input field.
@toddjordan
Copy link
Contributor

Works in the super rentals repo: https://github.com/ember-learn/super-rentals/blob/master/tests/integration/components/list-filter-test.js#L65

Maybe try that out and try to find the difference in what you are seeing.

@algo74
Copy link
Contributor Author

algo74 commented Mar 12, 2018

In the repo, the line 68 is
assert.ok(this.element.querySelector('.city'), 'one result found');
if replaced with correct
assert.equal(this.element.querySelectorAll('.city').length, 1, 'One result returned');
the test fails.

My solution is probably not very good though. For instance, fillIn is introduced in the next test on this page.

@toddjordan
Copy link
Contributor

it seems like the assert.ok would be ok, since I believe it gives back undefined if it can't find the selector. Will have to investigate though.

@algo74
Copy link
Contributor Author

algo74 commented Mar 12, 2018

The test must assert that the component shows exactly one item. A query with empty string returns three items (it is tested in the first test).

@toddjordan
Copy link
Contributor

You're right. Thanks @algo74 ❤️

@toddjordan toddjordan merged commit 6b05b02 into emberjs:master Mar 14, 2018
@algo74 algo74 deleted the patch-3 branch March 14, 2018 23:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants