Add a new test case for E2E test (Show admin bar if user logged in)

[hideokamoto]

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

[hideokamoto]

To add the test case

[talldanwp]

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 for e2e-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 to expect( nodes.length ).toBe( 1 );, it's also safe to use the stricter toBe 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!

[hideokamoto]

Update patch regarding to the feedback

[hideokamoto]

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.

[talldanwp]

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 );
} );

[hideokamoto]

New patch file for the ticket

[hideokamoto]

@talldanwp
I'm sorry for the late reply.

I've tried your code and it works fine!
I've updated the patch now.

Thanks.