test(search): improve test coverage

[oscard0m]

📝 Summary

• Improve test coverage
• ⚠️ still one line not covered:
• L26
• L65

⛱ Motivation and Context

To reach 100% test coverage

[jsumners]

Is this still in draft or are you okay with not covering line 65?

[oscard0m]

Is this still in draft or are you okay with not covering line 65?

I was expecting to land this since I did not find an easy way to cover that line:

alfred-emoji/src/search.js

Lines 61 to 69 in d054a71

	const alfredItems = (names) => {
	const items = []
	names.forEach((name) => {
	const emoji = emojilib.lib[name]
	if (!emoji) return
	items.push(alfredItem(emoji, name))
	})
	return { items }
	}

It depends on emojilib and I did not find a way to inject or mock with tap so I can force that path of execution. Maybe you have more experience with it and you can provide some guidance here? 👼🏽

[jsumners]

I would use https://www.npmjs.com/package/mock-require

[oscard0m]

I would use npmjs.com/package/mock-require

@jsumners

Working with your package proposal and so far so good. With current mocked emojilib I'm facing problems giving test coverage for the following lines:

Coverage for Line 26

alfred-emoji/src/search.js

Lines 23 to 27 in d054a71

	const addModifier = (emoji, modifier) => {
	if (!modifier || !emoji.fitzpatrick_scale) return emoji.char
	const zwj = new RegExp('‍', 'g')
	return emoji.char.match(zwj) ? emoji.char.replace(zwj, modifier + '‍') : emoji.char + modifier
	}

Checking the code and the possible values in emojisMock.json, char property will always be a string, and, consequently, the .match expression will always be truthy, right? Checking the commit adding this lines did not give to me any clue on what is this logic for. Any clue?

(strange because with previous version of my tests I made it to cover this line but I don't know exactly how 🙄)

Coverage for Line 65

alfred-emoji/src/search.js

Lines 61 to 80 in d054a71

	const alfredItems = (names) => {
	const items = []
	names.forEach((name) => {
	const emoji = emojilib.lib[name]
	if (!emoji) return
	items.push(alfredItem(emoji, name))
	})
	return { items }
	}
	const all = () => alfredItems(emojiNames)
	const matches = (terms) => {
	return emojiNames.filter((name) => {
	return terms.every((term) => {
	return name.includes(term) ||
	emojilib.lib[name].keywords.some((keyword) => keyword.includes(term))
	})
	})
	}

According to the invocation of matches() and after it, alfredItems

alfred-emoji/src/search.js

Line 94 in d054a71

return alfredItems(matches(terms))

I think there is no path of execution where line 65 can be falsy so that it stops execution there. Could that be possible? If so, do you have any example where that could happen, so I can reproduce it in the mock?

Thanks for your support and help here :)

[jsumners]

Checking the code and the possible values in emojisMock.json, char property will always be a string, and, consequently, the .match expression will always be truthy, right? Checking the commit adding this lines did not give to me any clue on what is this logic for. Any clue?

Maybe @Glennmen can help answer this question. It was added by 2973921

I think there is no path of execution where line 65 can be falsy so that it stops execution there.

I believe the thinking is that if one searches for "baz" then it will short circuit and not attempt to add any search results for Alfred to display.

[Glennmen]

The logic I got for the skintone modifier came from: https://github.com/muan/mojibar/blob/3a1dac0998202e8425a63acb37da82aa7d6385f9/app/search.js#L225

Not sure about line 65 through, can't remember if I actually checked if that is ever triggered.
Anyways this test should catch if a none valid search is passed:

alfred-emoji/test/search.test.js

Lines 24 to 28 in d054a71

	test('omits "rage1"', (t) => {
	t.plan(1)
	const found = search('rage1')
	t.ok(Object.keys(found.items).length === 0)
	})

It has been a while sorry 😅 if there is something else I can help with please let me know.

[oscard0m]

Finally found the use case for L26 🥳
Here is the key commit to find it: muan/mojibar@b4c1f47
And here is the info I found about ZWJ emojis: https://emojipedia.org/emoji-zwj-sequence/

[oscard0m]

To make test to cover L65 and do not fail for the rest I had to add an extreme use case:

• a term appearing in orderedList.json and not appearing in emojiLib.json

To avoid exploding for the rest of tests, I had to add an extra defensive programming statement:

emojilib.lib[name] &&
    emojilib.lib[name].keywords.some((keyword) => keyword.includes(term))

[jsumners]

Fantastic. Thank you for sticking with this.