Social Icons - Begin Automating Existing Manual Test Cases. #39027
Conversation
ttahmouch
added
the
[Block] Social
ttahmouch
requested review from
ellatrix,
cameronvoell,
ntwb,
nerrad and
ajitbohra
as code owners
ttahmouch
removed request for
ntwb,
ajitbohra,
cameronvoell,
nerrad and
ellatrix
…ial Icons, e.g., which Social Icons are rendered, in/active, hyperlinked, etc. The manual tests being referenced come from here: https://github.com/wordpress-mobile/test-cases/blob/trunk/test-cases/gutenberg/social-icons.md#tc001
ttahmouch
force-pushed
the
feature/automate-test-cases
branch
from
8088574
to
2604748
Compare
|
Size Change: -244 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
|
Hey @ttahmouch 👋, I see you requested a review for this PR but looks like it's still in draft, just wanted to check whether is ready or if you're looking for something specific to get reviewed before marking it as ready for review 🤔 . |
Hey, @fluiddot . I marked it as I may remove the mocked hook in |
…active icon styles to a constant defined closer to the styles import, and mentioned where the values are derived. The rest of the changes are unrelated by came from a `npm run lint-js:fix`.
…the "global mocks" as the "native mocks," i.e., `test/native/setup.js` are the only relevant ones for the component.
| @@ -493,12 +493,12 @@ SPEC CHECKSUMS: | |||
| React-logger: faee236598b0f7e1a5e3b68577016ac451f1f993 | |||
There was a problem hiding this comment.
These changes were generated. They are unrelated to the implementation details of the component being tested, or the tests.
| @@ -36,9 +36,9 @@ export function finishResolution( selectorName, args ) { | |||
| * Returns an action object used in signalling that a batch of selector resolutions has | |||
There was a problem hiding this comment.
These changes were generated by npm run lint-js:fix. They are unrelated to the implementation details of the component being tested, or the tests.
| @@ -25,8 +25,8 @@ export type HigherOrderComponent< HOCProps extends Record< string, any > > = < | |||
| * Given a function mapping a component to an enhanced component and modifier | |||
There was a problem hiding this comment.
These changes were generated with npm run lint-js:fix. They are unrelated to the implementation details of the component being tested, or the tests.
| @@ -19364,16 +19364,6 @@ | |||
| "yauzl": "^2.7.0" | |||
| }, | |||
| "dependencies": { | |||
| "are-we-there-yet": { | |||
There was a problem hiding this comment.
These changes were generated. They are unrelated to the implementation details of the component being tested, or the tests.
test/native/setup.js
Outdated
| @@ -222,6 +222,12 @@ jest.mock( '@wordpress/compose', () => { | |||
| mockComponent( 'ResizeObserverMock' )( {} ), | |||
| { width: 100, height: 100 }, | |||
| ] ), | |||
| usePreferredColorSchemeStyle: jest.fn( () => { | |||
There was a problem hiding this comment.
These mock values are effectively "arbitrary," but were taken from values like $light-quaternary in gutenberg/packages/base-styles/_colors.native.scss
| */ | ||
| it( 'should display WORDPRESS, FACEBOOK, TWITTER, INSTAGRAM by default.', async () => { | ||
| // Arrange | ||
| const subject = await initializeEditor( {} ); |
There was a problem hiding this comment.
The staging in each of these tests is purposefully redundant in an effort to keep the code easy to understand in isolation. I don't like DRY code in test suites personally, but the thought crossed my mind about checking for existing utils or creating one. It just doesn't feel worth it at the moment.
| @@ -0,0 +1,138 @@ | |||
| /** | |||
There was a problem hiding this comment.
The tests written for the Edit Social Links Block type started to be derived from here:
https://github.com/wordpress-mobile/test-cases/blob/trunk/test-cases/gutenberg/social-icons.md#tc001
They're just rudimentary at the moment to start incrementally, and keep the size of the PR down to easily discuss them. I planned to add more tests in future PRs when I started this.
|
I just noticed some test failures unrelated to the tests in this PR because the snapshot assertions they are running were affected by the default active icon styles I added in the https://github.com/WordPress/gutenberg/runs/5324097673?check_suite_focus=true |
|
@ttahmouch The changes look good to me. Your reasoning to keep avoid going DRY at this point also makes sense to me. I ran the tests locally and they all passed. I did get this ugly warning : but it didn't break anything. I'm guessing I have something off on my dev environment. |
❤️ 🙏 Thank you so much for taking a look, @jhnstn . I appreciate it a lot. I'm glad you feel the same way regarding DRY.
I did get that ugly warning as well, but it didn't impact the test assertions since I saw them go from red to green when TDDing it. I thought about spending some time trying to fix whatever was wrong with the |
Agreed. Doesn't seem to block this work so no need to fix it here. |
…st having default inactive icon styles in the `<SocialLinkEdit/>` component to avoid breaking snapshot assertions.
|
All of the tests and workflows are ✅ now, @jhnstn . If you have some time soon, would you mind giving the remaining changes a cursory glance? They're fairly small. |
I agree, and I appreciate your input and the review. It means a lot to me. I just wanted to time box the changes for this specific PR, and addressing the snapshot assertions was taking longer than I thought was reasonable at the moment. This just felt like a good compromise. |
|
Yay, cool to see more automated tests merged! @ttahmouch , for easier reference, can you call out in the PR description exactly which tests (titles) have been added by the PR? And more interestingly, can we drop those from the manual testsuites or there are parts that are performed in manual but not yet in the automated tests so we need to keep those manual for now? |
priethor
added
the
[Type] Automated Testing
Description
This is a first pass at beginning to automate some of the manual regression test cases for the Social Icons block types that we assert during some of our release rotations.
They are derived from here:
https://github.com/wordpress-mobile/test-cases/blob/trunk/test-cases/gutenberg/social-icons.md#tc001
The couple tests in this PR cover most of the assertions from this test:
They assert that, by default, WordPress, Facebook, Twitter, and Instagram icons are add added when Social Icons are added. They also assert that WordPress contains a link making it active by default, and the remaining three do not contain a link making them inactive by default. The icon color for the active and inactive icons hasn't been asserted, but the state of activity has been.
I think that when asserting the UX, checking the active state of each of the Social Icons by default makes sense, but I see little value in asserting the color of the icons in each state. That being said, if we want to remain faithful to the original manual test, asserting the colors of the icons would be necessary before the manual test could be removed.
Testing Instructions
Screenshots
N/A
Types of changes
New "feature." Adding automated integration tests.
Checklist:
*.native.jsfiles for terms that need renaming or removal).