Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

bugITwhisperer: playwright skeleton tests steps update (Issue #378) #379

Conversation

bugITwhisperer
Copy link
Contributor

@bugITwhisperer bugITwhisperer commented Oct 3, 2023

Fixes Issue #378

Changes proposed

  • removed: 2 Playwright test skeletons which are already covered in the tests above and no work is required. Since they are redundant, it's better to delete them not to cause any confusion to anyone reading the code
  • added: empty lines between actions to maintain the consistency between introduction.spec.ts and login.spec.ts files
  • adjusted/changed a bit: tests' names for a better readability
  1. This 1st skeleton does not require fixing:
test.fixme('is the highlighted item in the left-menu is the correct one', async ({ page }) => {
  // navigate to the docs landing page /docs
  // check if the Introduction element in the left menu is holding the active class
});

since it's already covered in the 1st test:

test('Docs header element is active', async ({ page }) => {
  // navigate to the docs landing page /docs
  await page.goto('/docs');

  // check if the docs link in the page header is having the active class
  await expect(page.getByRole('link', { name: 'Docs' })).toHaveClass(/active/);
});
  1. Same for the 2nd skeleton:
test.fixme(
  'is the left-side menu containing an element with the same name as the page headline',
  async ({ page }) => {
    // navigate to the docs landing page /docs
    // check if the Introduction element in the left menu is holding the active class
  },
);

it's been covered by this test:

test('Page Heading is correct', async ({ page }) => {
  // navigate to the docs landing page /docs
  await page.goto('/docs');

  // check if the heading contains Introduction
  expect(await page.textContent('h1')).toBe('Introduction');
});

@bugITwhisperer
Copy link
Contributor Author

@eddiejaoude, @Benmuiruri , @frazie , @tbhaxor: hi there! could you have a look at my suggested changes? :)

Comment on lines -27 to -28
// navigate to the docs landing page /docs
// check if the Introduction element in the left menu is holding the active class
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh I think the test case is valid, you mean the test description is the same one for the ones above right? In case yes, we should definitely check the steps for testing it!

Copy link
Contributor Author

@bugITwhisperer bugITwhisperer Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cahllagerfeld : well, yeah. The lines 27-28 are duplicates of lines 21-22, they say the very same thing and do not match the test description at all:

  // navigate to the docs landing page /docs
  // check if the Introduction element in the left menu is holding the active class

Copy link
Contributor Author

@bugITwhisperer bugITwhisperer Oct 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you confirm the proper steps in those skeletons should be as follows?

test.fixme('is the highlighted item in the left-menu is the correct one', async ({ page }) => {
  // navigate to the docs landing page /docs
  // check if the Introduction element in the left menu is highlighted element
});
test.fixme(
  'is the left-side menu containing an element with the same name as the page headline', async ({ page }) => {
    // navigate to the docs landing page /docs
    // check if both element in the left menu and page header are the same and say 'Introduction'
 },
);

@bugITwhisperer bugITwhisperer changed the title bugITwhisperer: playwright skeleton tests duplicates removal bugITwhisperer: playwright skeleton tests steps update Oct 4, 2023
Copy link
Member

@Cahllagerfeld Cahllagerfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that it's looking good to me 👍

tests/docs/introduction.spec.ts Outdated Show resolved Hide resolved
@bugITwhisperer
Copy link
Contributor Author

@Cahllagerfeld : can you add hacktoberfest and/or hacktoberfest-accepted labels before merging this PR?

@eddiejaoude eddiejaoude added the hacktoberfest-accepted This PR has been marked as qualifying for Hacktoberfest. label Oct 6, 2023
@eddiejaoude
Copy link
Member

eddiejaoude commented Oct 6, 2023

can you add hacktoberfest and/or hacktoberfest-accepted labels before merging this PR?

I have added the label hacktoberfest-accepted (but it is not required because the whole repo has the topic hacktoberfest)

Great collab 💪

@Cahllagerfeld Cahllagerfeld merged commit 0bde86c into EddieHubCommunity:main Oct 6, 2023
2 checks passed
@bugITwhisperer bugITwhisperer deleted the bugITwhisperer/playwright-tests-update branch October 7, 2023 10:56
@bugITwhisperer bugITwhisperer changed the title bugITwhisperer: playwright skeleton tests steps update bugITwhisperer: playwright skeleton tests steps update #378 Oct 8, 2023
@bugITwhisperer bugITwhisperer changed the title bugITwhisperer: playwright skeleton tests steps update #378 bugITwhisperer: playwright skeleton tests steps update (Issue #378) Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
hacktoberfest-accepted This PR has been marked as qualifying for Hacktoberfest.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants