Testing: Add missing unit tests to packages

[gziolo]

Raised by @georgestephanis on WordPress.org Slack (link requires registration at https://make.wordpress.org/chat/):
https://wordpress.slack.com/archives/C02QB2JS7/p1549826915372300

Some of the students I'm mentoring like Ashwin are interested in contributing more unit tests to Gutenberg -- is there any list somewhere of areas that need better test coverage?

There is no list like this. There used to be a similar task in the past but we closed it as it became very outdated. I can't find the link at the moment.

This is going to be a living document where we should collectively collect all API methods that are exposed in WordPress packages but aren't unit tested. I would prefer to skip UI components in this issue as they are usually harder to cover with tests.

Please make sure to comment on the issue if you plan to work on adding unit tests. Remember about sharing all details of the work planned to avoid duplication of efforts. Thanks!

1. @wordpress/autop
   • removep (more details in autop: Include tests for removep functionality #15152)
2. @wordpress/blob
   • createBlobURL
   • getBlobByURL
   • revokeBlobURL
3. @wordpress/compose
   • ifCondition (complex)
   • withSafeTimeout (complex)
4. @wordpress/date
   • format (complex)
5. @wordpress/dom
   • computeCaretRect
   • documentHasSelection
   • getOffsetParent
   • getScrollContainer
   • getRectangleFromRange
   • isEntirelySelected
   • isVerticalEdge
   • placeCaretAtVerticalEdge
   • tabbable.isTabbableIndex
6. @wordpress/keycodes
   • isKeyboardEvent (Added testcases for isKeyboardEvent in keycodes module #14073)
7. @wordpress/priority-queue
   • createQueue (complex++)

[dsab123]

I'd be happy to look at some of these! I've never used jest before 😍

I'm working on writing a test for createBlobURL() right now. I'll probably end up using something like babel-rewire-plugin to mock some non-exported properties I'd like to get access to, if that's alright. Or maybe there's a better way?

const cache = {};

export function createBlobURL( file ) {
	const url = createObjectURL( file );

	cache[ url ] = file;

	return url;
}

[gziolo]

I'll probably end up using something like babel-rewire-plugin to mock some non-exported properties I'd like to get access to, if that's alright. Or maybe there's a better way?

Is that necessary to verify internals through mocking library? I think getBlobByURL operates on cache only, so you can use it as a way to access the internal cache.

[dsab123]

Well, that would work. But then the test does not strictly test the functionality of createBlobURL(). The test could fail in the future if getBlobByURL() changes in some way, say, no longer writes to the cache.

What's the likelihood of getBlobByURL() changing? If its not likely then I can test createBlobURL() using it, but it still makes me a bit uncomfortable..

[gziolo]

Well, that would work. But then the test does not strictly test the functionality of createBlobURL(). The test could fail in the future if getBlobByURL() changes in some way, say, no longer writes to the cache.

Those methods aren't pure and therefore are hard to test. I wouldn't spend too much time on it. Very basic verification is fine as this is a tiny wrapper over URL API.

[dsab123]

👍 okie doke! I'll get a few tests in for blob sometime this afternoon or tomorrow. Thanks @gziolo !

[dsab123]

I think this is related: https://stackoverflow.com/a/52969731 and jestjs/jest#7645; thinking of some possible solutions.

In a nutshell: jest ships with jsdom, which does not have support for URL.createObjectURL and URL.revokeObjectURL as per jsdom/jsdom#1721

[ashwin-pc]

Thanks for the list @gziolo

Ill take a stab at the keycodes test case first.

[gziolo]

I don't see any movement on this issue. Let's close this one and work on adding more unit tests as we update the functionality rather than independently from production changes.