google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 291 forks source link

Test logic within notification registration #9273

Open jimmymadon opened 2 months ago

jimmymadon commented 2 months ago

Feature Description

In #8978 and #8979, the ErrorNotifications.test.js require "mock" registration of notifications using the new Notifications datastore register function.

https://github.com/google/site-kit-wp/blob/dc0f77f42b41fa93326ebaa56c473eb2b729726f/assets/js/components/notifications/ErrorNotifications.test.js#L46-L73

This would require us to "copy-paste" code that registers notifications within our tests. In addition to copy-pasting, the main drawback here is that our tests do not actually cover the real production registration code within register-defaults.js. So we need a way to cover the logic within this code. We could perhaps create a utility function that wraps production registration code and adds it to a test registry.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

tofumatt commented 2 months ago

ACs here make sense, moving to IB 🙂

eugene-manuilov commented 2 months ago

IB ✔️

jimmymadon commented 1 month ago

I have bumped up the estimate here as this issue will have to fix tests from another newly refactored notification - #9280. Have updated the AC. The IB won't change here - but more work will have to be done in Execution.