Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Baseline tests for image picker component #34622

Merged
merged 3 commits into from May 6, 2020
Merged

Conversation

bencodeorg
Copy link
Contributor

Add some basic tests for the imagePicker component, which is used in Applab to pick an image:

image

Pre-work for updates to imagePicker to show students it's possible to add an image directly from a URL.

Testing story

New tests passed locally.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@bencodeorg bencodeorg requested a review from islemaster May 5, 2020 17:27
@bencodeorg
Copy link
Contributor Author

bencodeorg commented May 5, 2020

I also feel like there's testing missing here that confirms that the modal opens when a user clicks "Choose..." in the imagePickerPropertyRow component, and closes once an asset is chosen:

image

Neither imagePickerPropertyRow (the component that represents where you enter an image URL and the "Choose..." link, or the show.js file that actually is responsible for showing the modal have tests. Thoughts on where it would be appropriate to add such testing?

@islemaster
Copy link
Contributor

Both of those files could probably use unit tests, but both are also tied into some global state which might be why they don't have tests now. I'd start with ImagePickerPropertyRow and just check that it makes the expected call to dashboard.assets.showAssetManager when you click the "Choose" link.

I don't think that should block this change though - these new tests look good on their own.

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding coverage here! Looks like the four tests you added bring coverage to 83% for ImagePicker.jsx, which is great for initial coverage.

@bencodeorg bencodeorg merged commit 4b151cf into staging May 6, 2020
@bencodeorg bencodeorg deleted the image-picker-tests branch May 6, 2020 17:33
@bencodeorg
Copy link
Contributor Author

For my knowledge, how did you get that test coverage metric?

@islemaster
Copy link
Contributor

Generate a coverage report by prefixing COVERAGE=1 to any JS test command:

COVERAGE=1 yarn test:entry --entry=test/unit/code-studio/components/ImagePickerTest.js

This generates an HTML report in [cdo]/apps/coverage that you can browse.

# Ubuntu
google-chrome coverage/entry/index.html
# OSX
open coverage/entry/index.html

You'll enter at a top-level view of the apps directory, and since in this example we only ran the one test file, coverage will look pretty dismal. In fact, this report won't include files that weren't touched at all by this test run, so don't be surprised if it seems like folders are missing.

image

But if you drill down you can find the particular file you intended to test:

image

You can even open up the file and view line-by-line coverage:

image

@islemaster
Copy link
Contributor

Because it may be interesting: I ran our full unit test suite today (no storybook or integration tests) with coverage, and it reports 54% of statements covered right now. That varies a lot for different parts of our code, and is including some third-party code that is copied into our repo.

Here is the coverage of some sibling components to ImagePicker in code-studio/components:

image

@bencodeorg bencodeorg changed the title Default tests for image picker component Baseline tests for image picker component May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants