Add unit tests for Select component

[HenryRocha]

Related to #376

Add unit tests for Select component.

Technical details

Add test checking if the trigger button is rendered with the correct text.
Add test to check if custom classes and primary appearance are applied correctly.
Add test to check if all options are rendered.
Add test to check if currently selected option updates correctly. (Not working)

Observations:
The last test, which checks if the selected option updates correctly, is not working. Not sure if it should be working or if we need to mock the function which updates the selected value. Some help would be very welcome.

Also, please check if these tests are enough or if I missed anything.

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the master branch of the repository
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no
  visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Henry Rocha

[pavish]

@HenryRocha Thank you, the PR looks great!

I would recommend adding more tests for:

• The dropdown list highlights the currently selected option
• Keyboard and ARIA functions
  • Movement through list using up and down arrows
  • Selection by using enter
  • Hiding the dropdown using esc
• The list scrolls automatically to display the currently selected option

I think we can merge this PR first and a new PR can be raised with these tests.

I'll also defer to @seancolsen and @mr-gabe49 (he built this Select component) for their comments, if they have any.

[HenryRocha]

@pavish thank you! I'll wait until the creators of the component give their opinions before working on more tests then. In the mean time, I'll try fixing the test to check if currently selected option updates correctly.

[mr-gabe49]

@pavish thank you! I'll wait until the creators of the component give their opinions before working on more tests then. In the mean time, I'll try fixing the test to check if currently selected option updates correctly.

@HenryRocha This looks great! Thanks for your work! @pavish comments have everything covered.

[HenryRocha]

@pavish I've added a few more tests, covering most of the one you mentioned:

• The dropdown list highlights the currently selected option
• Keyboard and ARIA functions
  • Movement through list using up and down arrows
  • Selection by using enter
  • Hiding the dropdown using esc
• The list scrolls automatically to display the currently selected option

One observation: I tried using fireEvent.keyPress but for some reason that did not work, so I ended up using keyDown and keyUp.

Also, I looked around to see if it was possible to see what was being displayed, in order to check if the list scrolls automatically to display the currently selected option, but I could not find anything, so I left that test out. Any tips on how to approach this would be appreciated.

Please review the new tests and if they're enough, or if I missed anything.

[pavish]

@HenryRocha

One observation: I tried using fireEvent.keyPress but for some reason that did not work, so I ended up using keyDown and keyUp.

The KeyPress event is deprecated and the Select component does not handle it.

fireEvent does not actually perform the event, it instead fires the event to trigger listeners. This is the reason why fireEvent.keyPress did not work. We do not have a test utility to actually perform the keyboard action (which actually triggers all these events: keyPress, keyUp and keyDown).

I think it's okay to use fire only keyDown and keyUp for the tests.

Also, I looked around to see if it was possible to see what was being displayed, in order to check if the list scrolls automatically to display the currently selected option, but I could not find anything, so I left that test out. Any tips on how to approach this would be appreciated.

The testing library does not offer an option to do that (In all fairness, it is incredibly hard/impossible to implement a generic method by just being able to read the DOM).

We can test that for our particular usecase by reading the scrollTop value of the dropdown container and the offsetTop of the selected option element. If the following conditions are satisfied, that means the element is fully visible.

optionElement.offetTop > dropdownContainer.scrollTop
optionElement.offsetTop + optionElement.clientHeight < dropdownContainer.scrollTop + dropdownContainer.clientHeight

[pavish]

@HenryRocha This looks great! I only have a couple minor comments.

Regarding the test case for 'The list scrolls automatically to display the currently selected option', please feel free to decide whether you'd like to include it in this PR or take it up separately. I am okay with merging this PR without it.

[HenryRocha]

@pavish I've fixed the issues you mentioned and merged in the new changes from master.

As for the missing tests, I'd say merge the current changes and we'll take up the missing tests in another PR.

[mathemancer]

@pavish I've fixed the issues you mentioned and merged in the new changes from master.

As for the missing tests, I'd say merge the current changes and we'll take up the missing tests in another PR.

Is this ready for re-review? If so, please re-request review via the button next to @pavish 's username.

[pavish]

@HenryRocha This PR looks great to me!