Opened 14 months ago
Last modified 5 weeks ago
#48978 new enhancement
Add a new test case for E2E test (Show admin bar if user logged in)
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | needs-patch |
Focuses: | javascript | Cc: |
Description
We can add a new test case for our e2e test.
Case
- When: User logged in.
- Where: Go to own website.
- Expect: Should show admin bar top of the page
Attachments (3)
Change History (10)
#2
@
12 months ago
- Keywords needs-patch added
Hi @hideokamoto, Thanks for adding an end to end test. I thought I'd share some feedback about the test:
- It'd be great if you could add semi-colons as per the coding standards (https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#semicolons)
- Currently there's a separate import for
loginUser
, this should ideally be part of import on the line above fore2e-test-utils
so that it looks like this:import { visitAdminPage, createURL, loginUser } from '@wordpress/e2e-test-utils';
expect( nodes.length ).not.toEqual( 0 );
I know the test above has a similar expectation, but as far as I'm aware more than one admin bar would be unexpected. I think it'd be better to change this toexpect( nodes.length ).toBe( 1 );
, it's also safe to use the strictertoBe
matcher for numbers.- I think it could be worth also testing for absence of the admin bar when logged out, this would likely be a separate test case.
- What do you think about changing the test description from 'Login User' to 'Admin Bar'? I think that would better reflect what's being tested.
Thanks!
#3
@
12 months ago
Thanks for the feedback!
I've updated the patch today.
expect( nodes.length ).not.toEqual( 0 ); I know the test above has a similar expectation, but as far as I'm aware more than one admin bar would be unexpected. I think it'd be better to change this to expect( nodes.length ).toBe( 1 );, it's also safe to use the stricter toBe matcher for numbers.
I've replaced the assertion from expect( nodes.length ).not.toEqual( 0 )
to expect( ! ! ( nodes ) ).toBe( true )
.
Because, the assertion expect( nodes.length ).toBe( 1 )
not works well.
> node ./tests/e2e/run-tests.js FAIL tests/e2e/specs/hello.test.js Hello World ✓ Should load properly (1814ms) Admin Bar ✕ Should show admin bar when user logged in (1021ms) ● Admin Bar › Should show admin bar when user logged in TypeError: Cannot read property 'length' of null 15 | await page.goto( createURL( '/' ) ); 16 | const nodes = await page.$( '#wpadminbar1' ); > 17 | expect( nodes.length ).toBe( 1 ); | ^ 18 | } ); 19 | } ); at Object.<anonymous> (specs/hello.test.js:17:17) Test Suites: 1 failed, 1 total Tests: 1 failed, 1 passed, 2 total Snapshots: 0 total Time: 4.018s Ran all test suites.
So I change the code to check just the element exists.
Pass case
describe( 'Admin Bar', () => { it( 'Should show admin bar when user logged in' , async () => { await loginUser(); await page.goto( createURL( '/' ) ); const nodes = await page.$( '#wpadminbar' ); expect( ! ! ( nodes ) ).toBe( true ); } ); } ); Admin Bar ✓ Should show admin bar when user logged in (1041ms)
Failure case
describe( 'Admin Bar', () => { it( 'Should show admin bar when user logged in' , async () => { await loginUser(); await page.goto( createURL( '/' ) ); const nodes = await page.$( '#wpadminbar111' ); expect( ! ! ( nodes ) ).toBe( true ); } ); } ); Admin Bar ✕ Should show admin bar when user logged in (802ms) ● Admin Bar › Should show admin bar when user logged in expect(received).toBe(expected) // Object.is equality Expected: true Received: false 15 | await page.goto( createURL( '/' ) ); 16 | const nodes = await page.$( '#wpadminbar111' ); > 17 | expect( ! ! ( nodes ) ).toBe( true ); | ^ 18 | } ); 19 | } ); at Object.<anonymous> (specs/hello.test.js:17:27)
I think it could be worth also testing for absence of the admin bar when logged out, this would likely be a separate test case.
I think so too.
But currently, there is a no helper function to logout the dashboard.
So probably, we need to create a helper function to logout the dashboard.
Kindly regards.
#4
@
11 months ago
Hi @hideokamoto,
Thanks for updating the patch. I see what's happening. It looks like the reason checking the length is not working is due to the selector using the page.$
function, which is the equivalent of document.querySelector
and only returns the first element it encounters.
If you use page.$$
(document.querySelectorAll
) instead the test seems to work well:
it( 'Should show admin bar when user logged in' , async () => { await loginUser(); await page.goto( createURL( '/' ) ); const nodes = await page.$$( '#wpadminbar' ); expect( nodes.length ).toBe( 1 ); } );
To add the test case