ronantakizawa/codereview-bench huggingface dataset:
Repository: freeCodeCamp/freeCodeCamp
File: e2e/four-oh-four.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.beforeEach(async ({ page }) => {
  await page.goto('/404');
});

test.describe('test for FourOhFour component', () => {
  test('should display 404 image', async ({ page }) => {
    const image = page.getByTestId('not-found-image');
    await expect(image).toBeVisible();
    await expect(image).toHaveAttribute(
      'alt',
      translations['404']['not-found']
    );
  });

  test('should display 404 heading', async ({ page }) => {
    const heading = page.getByTestId('not-found-heading');
    await expect(heading).toBeVisible();
    await expect(heading).toContainText(translations['404']['page-not-found']);
  });

  test('should display 404 heres-a-quote sentence', async ({ page }) => {
    const heresQuoteParagraph = page.getByTestId('heres-quote-paragraph');
    await expect(heresQuoteParagraph).toBeVisible();
    await expect(heresQuoteParagraph).toContainText(
      translations['404']['heres-a-quote']
    );
  });

  test('should display 404 quote', async ({ page }) => {
    await expect(page.getByTestId('quote-wrapper')).toBeVisible();

Review comment (suggestion):
Since this is a test suite, we don't need to have "Test" in the title.

```suggestion
test.describe('FourOhFour component', () => {
```

Code after:
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.beforeEach(async ({ page }) => {
  await page.goto('/404');
});

test.describe('FourOhFour component', () => {
  test('should display the content correctly', async ({ page }) => {
    const image = page.getByTestId('not-found-image');
    await expect(image).toBeVisible();
    await expect(image).toHaveAttribute(
      'alt',
      translations['404']['not-found']
    );

    const heading = page.getByTestId('not-found-heading');
    await expect(heading).toBeVisible();
    await expect(heading).toContainText(translations['404']['page-not-found']);

    const heresQuoteParagraph = page.getByTestId('heres-quote-paragraph');
    await expect(heresQuoteParagraph).toBeVisible();
    await expect(heresQuoteParagraph).toContainText(
      translations['404']['heres-a-quote']
    );

    await expect(page.getByTestId('quote-wrapper')).toBeVisible();

    const curriculumLinkBtn = page.getByTestId('view-curriculum-btn');
    await expect(curriculumLinkBtn).toBeVisible();
    await expect(curriculumLinkBtn).toHaveAttribute('href', '/learn');
    await expect(curriculumLinkBtn).toContainText(
      translations.buttons['view-curriculum']
Repository: freeCodeCamp/freeCodeCamp
File: e2e/four-oh-four.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.beforeEach(async ({ page }) => {
  await page.goto('/404');
});

test.describe('test for FourOhFour component', () => {
  test('should display 404 image', async ({ page }) => {
    const image = page.getByTestId('not-found-image');
    await expect(image).toBeVisible();
    await expect(image).toHaveAttribute(
      'alt',
      translations['404']['not-found']
    );
  });

  test('should display 404 heading', async ({ page }) => {
    const heading = page.getByTestId('not-found-heading');
    await expect(heading).toBeVisible();
    await expect(heading).toContainText(translations['404']['page-not-found']);
  });

  test('should display 404 heres-a-quote sentence', async ({ page }) => {
    const heresQuoteParagraph = page.getByTestId('heres-quote-paragraph');
    await expect(heresQuoteParagraph).toBeVisible();
    await expect(heresQuoteParagraph).toContainText(
      translations['404']['heres-a-quote']
    );
  });

  test('should display 404 quote', async ({ page }) => {
    await expect(page.getByTestId('quote-wrapper')).toBeVisible();
  });

Review comment (bug):
We should group all of the tests into one: "should display the content correctly" or something similar. 

There isn't anything wrong with the tests, but splitting them would be more appropriate if we are writing unit tests. Since we are writing end-to-end tests here instead, which are expensive to run, we would want to be a little more mindful of the execution.

Code after:
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.beforeEach(async ({ page }) => {
  await page.goto('/404');
});

test.describe('FourOhFour component', () => {
  test('should display the content correctly', async ({ page }) => {
    const image = page.getByTestId('not-found-image');
    await expect(image).toBeVisible();
    await expect(image).toHaveAttribute(
      'alt',
      translations['404']['not-found']
    );

    const heading = page.getByTestId('not-found-heading');
    await expect(heading).toBeVisible();
    await expect(heading).toContainText(translations['404']['page-not-found']);

    const heresQuoteParagraph = page.getByTestId('heres-quote-paragraph');
    await expect(heresQuoteParagraph).toBeVisible();
    await expect(heresQuoteParagraph).toContainText(
      translations['404']['heres-a-quote']
    );

    await expect(page.getByTestId('quote-wrapper')).toBeVisible();

    const curriculumLinkBtn = page.getByTestId('view-curriculum-btn');
    await expect(curriculumLinkBtn).toBeVisible();
    await expect(curriculumLinkBtn).toHaveAttribute('href', '/learn');
    await expect(curriculumLinkBtn).toContainText(
      translations.buttons['view-curriculum']
    );
Repository: freeCodeCamp/freeCodeCamp
File: e2e/form-helper.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.use({ storageState: 'playwright/.auth/certified-user.json' });

test.afterEach(async ({ page }) => {
  await page.close();
});

test.describe('Test form with only solution link', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(
      'learn/front-end-development-libraries/front-end-development-libraries-projects/build-a-random-quote-machine'
    );
  });

  test('The form is visible and has only solution link input', async ({
    page
  }) => {
    // The solution form should be present and visible
    const solutionForm = page.getByTestId('form-helper-form');
    await expect(solutionForm).toBeVisible();

    // The form submit button should be disabled as the form is not filled
    const solutionFormButton = solutionForm.getByRole('button', {
      name: `${translations.learn['i-completed']}`
    });
    await expect(solutionFormButton).toBeVisible();
    await expect(solutionFormButton).toBeDisabled();

    const solutionLinkInputLabel = solutionForm.getByTestId(
      'solution-control-label'
    );

Review comment (suggestion):
```suggestion
```

We don't need this - playwright does it automatically.

Code after:
import { test, expect } from '@playwright/test';
import translations from '../client/i18n/locales/english/translations.json';

test.use({ storageState: 'playwright/.auth/certified-user.json' });

test.describe('Test form with only solution link', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(
      'learn/front-end-development-libraries/front-end-development-libraries-projects/build-a-random-quote-machine'
    );
  });

  test('The form is visible and has only solution link input', async ({
    page
  }) => {
    // The solution form should be present and visible
    const solutionForm = page.getByTestId('form-helper-form');
    await expect(solutionForm).toBeVisible();

    // The form submit button should be disabled as the form is not filled
    const solutionFormButton = solutionForm.getByRole('button', {
      name: `${translations.learn['i-completed']}`
    });
    await expect(solutionFormButton).toBeVisible();
    await expect(solutionFormButton).toBeDisabled();

    const solutionLinkInputLabel = solutionForm.getByTestId(
      'solution-control-label'
    );
    await expect(solutionLinkInputLabel).toBeVisible();
    await expect(solutionLinkInputLabel).toHaveText(
      translations.learn['solution-link']
    );
Repository: freeCodeCamp/freeCodeCamp
File: e2e/landing.spec.ts (TypeScript)

Code before:
test('Testimonial endorser people have images, occupation, location and testimony visible', async () => {
  const cards = page.getByTestId('testimonial-card');
  await expect(cards).toHaveCount(3);
  for (const card of await cards.all()) {
    await expect(card).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-image-container')
    ).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-location')
    ).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-occupation')
    ).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-testimony')
    ).toBeVisible();
  }
});

test('Has links to all superblocks', async () => {
  const curriculumBtns = page.getByTestId(landingPageElements.curriculumBtns);
  await expect(curriculumBtns).toHaveCount(15);
  for (let item = 0; item < superBlocks.length; item++) {
    const btn = curriculumBtns.nth(item);
    await expect(btn).toContainText(superBlocks[item]);
  }
});

test('Has FAQ section', async () => {
  const faqs = page.getByTestId(landingPageElements.faq);
  await expect(faqs).toHaveCount(9);
});


Review comment (suggestion):
```suggestion
  for (let index = 0; index < superBlocks.length; index++) {
    const btn = curriculumBtns.nth(index);
    await expect(btn).toContainText(superBlocks[index]);
```

Nitpick, but if we're going to deviate from the standard `i` for loop iterators, we should at least name it correctly.

Code after:
test('Testimonial endorser people have images, occupation, location and testimony visible', async () => {
  const cards = page.getByTestId('testimonial-card');
  await expect(cards).toHaveCount(3);
  for (const card of await cards.all()) {
    await expect(card).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-image-container')
    ).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-location')
    ).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-occupation')
    ).toBeVisible();
    await expect(
      card.getByTestId('testimonials-endorser-testimony')
    ).toBeVisible();
  }
});

test('Has links to all superblocks', async () => {
  const curriculumBtns = page.getByTestId(landingPageElements.curriculumBtns);
  await expect(curriculumBtns).toHaveCount(15);
  for (let index = 0; index < superBlocks.length; index++) {
    const btn = curriculumBtns.nth(index);
    await expect(btn).toContainText(superBlocks[index]);
  }
});

test('Has FAQ section', async () => {
  const faqs = page.getByTestId(landingPageElements.faq);
  await expect(faqs).toHaveCount(9);
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/desktop-layout-component.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';

test.use({ storageState: 'playwright/.auth/certified-user.json' });

test.describe('Classic challengs - 3 pane desktop layout component', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(
      'learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
    );
  });

  test('The page has desktop layout with instructions/editor/preview pane', async ({
    page
  }) => {
    const desktopLayout = page.getByTestId('desktop-layout');
    await expect(desktopLayout).toBeVisible();

    const actionRow = desktopLayout.getByTestId('action-row');
    await expect(actionRow).toBeVisible();

    const tabsRow = desktopLayout.getByTestId('tabs-row');
    await expect(tabsRow).toBeVisible();

    const reflexContainer = desktopLayout.getByTestId('main-container');
    await expect(reflexContainer).toBeVisible();

    const instructionPane = desktopLayout.getByTestId('instruction-pane');
    await expect(instructionPane).toBeVisible();

    const editorPane = desktopLayout.getByTestId('editor-pane');
    await expect(editorPane).toBeVisible();

    const previewPane = desktopLayout.getByTestId('preview-pane');
    await expect(previewPane).toBeVisible();
  });

Review comment (suggestion):
There is a single test in this `describe` block, `beforeEach` hook is not really needed. It's ok if you decide  to leave it.

Code after:
import { test, expect } from '@playwright/test';

test.use({ storageState: 'playwright/.auth/certified-user.json' });

test.describe('Classic challenge - 3 pane desktop layout component', () => {
  test('The page has desktop layout with instructions/editor/preview pane', async ({
    page
  }) => {
    await page.goto(
      'learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
    );

    const desktopLayout = page.getByTestId('desktop-layout');
    await expect(desktopLayout).toBeVisible();

    const actionRow = desktopLayout.getByTestId('action-row');
    await expect(actionRow).toBeVisible();

    const tabsRow = desktopLayout.getByTestId('tabs-row');
    await expect(tabsRow).toBeVisible();

    const reflexContainer = desktopLayout.getByTestId('main-container');
    await expect(reflexContainer).toBeVisible();

    const instructionPane = desktopLayout.getByTestId('instruction-pane');
    await expect(instructionPane).toBeVisible();

    const editorPane = desktopLayout.getByTestId('editor-pane');
    await expect(editorPane).toBeVisible();

    const previewPane = desktopLayout.getByTestId('preview-pane');
    await expect(previewPane).toBeVisible();
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/editor-tabs.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
  );
});
test.afterEach(async ({ page }) => {
  await page.close();
});

test('should toggle editor visibility based on aria-expanded attribute', async ({
  page
}) => {
  // Find the first button that is not expanded (aria-expanded="false")
  const notExpandedButton = await page.$('button[aria-expanded="false"]');

  // If there's a button that's not expanded, click it
  if (notExpandedButton) {
    await notExpandedButton.click();

    // After the click, it should now have aria-expanded="true"
    const isExpandedAfterClick =
      await notExpandedButton.getAttribute('aria-expanded');
    expect(isExpandedAfterClick).toBe('true');
  }

  // Find the first button that is expanded (aria-expanded="true")
  const expandedButton = await page.$('button[aria-expanded="true"]');

  // If there's a button that's expanded, click it
  if (expandedButton) {
    await expandedButton.click();

    // After the click, it should now have aria-expanded="false"

Review comment (suggestion):
```suggestion
```

We actually don't need this. Playwright will close the page automatically for us.

Code after:
import { test, expect } from '@playwright/test';

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
  );
});

test('should toggle editor visibility correctly', async ({ page }) => {
  const htmlTabToggle = page.getByRole('button', { name: 'index.html Editor' });
  const cssTabToggle = page.getByRole('button', { name: 'styles.css Editor' });
  const htmlTab = page.getByTestId('editor-container-indexhtml');
  const cssTab = page.getByTestId('editor-container-stylescss');

  await expect(htmlTabToggle).toBeVisible();
  // HTML tab is opened by default
  await expect(htmlTabToggle).toHaveAttribute('aria-expanded', 'true');
  await expect(htmlTab).toBeVisible();
  await htmlTabToggle.click();
  await expect(htmlTabToggle).toHaveAttribute('aria-expanded', 'false');
  await expect(htmlTab).not.toBeVisible();

  await expect(cssTabToggle).toBeVisible();
  // CSS tab is not opened by default
  await expect(cssTabToggle).toHaveAttribute('aria-expanded', 'false');
  await expect(cssTab).not.toBeVisible();
  await cssTabToggle.click();
  await expect(cssTabToggle).toHaveAttribute('aria-expanded', 'true');
  await expect(cssTab).toBeVisible();
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/editor-tabs.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
  );
});

test('should toggle editor visibility based on aria-expanded attribute', async ({
  page
}) => {
  const tabLocator = page.locator('div.monaco-editor-tabs button').first();
  const isExpandedBeforeClick = await tabLocator.getAttribute('aria-expanded');
  if (isExpandedBeforeClick === 'true') {
    await tabLocator.click();
    const isExpandedAfterClick = await tabLocator.getAttribute('aria-expanded');
    expect(isExpandedAfterClick).toBe('false');
  } else {
    await tabLocator.click();
    const isExpandedAfterClick = await tabLocator.getAttribute('aria-expanded');
    expect(isExpandedAfterClick).toBe('true');
  }
});

test('toggling the button should toggle the appearance of the corresponding ReflexElement', async ({
  page
}) => {
  const tabsLocator = await page.locator('div.monaco-editor-tabs button').all();
  for (const tab of tabsLocator) {
    const tabFileKey = await tab.getAttribute('data-cy');
    if (tabFileKey) {
      const fileKey = tabFileKey.replace('editor-tab-', '');

      const ariaExpandedBeforeClick = await tab.getAttribute('aria-expanded');
      const reflexElementBeforeClick = page.locator(
        `div[data-cy="editor-container-${fileKey}"]`
      );

Review comment (suggestion):
Instead of querying the button using `first()`, we can query it using:

```ts
const button = page.getByRole('button', { name: 'button text' })
```

Code after:
import { test, expect } from '@playwright/test';

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
  );
});

test('should toggle editor visibility correctly', async ({ page }) => {
  const htmlTabToggle = page.getByRole('button', { name: 'index.html Editor' });
  const cssTabToggle = page.getByRole('button', { name: 'styles.css Editor' });
  const htmlTab = page.getByTestId('editor-container-indexhtml');
  const cssTab = page.getByTestId('editor-container-stylescss');

  await expect(htmlTabToggle).toBeVisible();
  // HTML tab is opened by default
  await expect(htmlTabToggle).toHaveAttribute('aria-expanded', 'true');
  await expect(htmlTab).toBeVisible();
  await htmlTabToggle.click();
  await expect(htmlTabToggle).toHaveAttribute('aria-expanded', 'false');
  await expect(htmlTab).not.toBeVisible();

  await expect(cssTabToggle).toBeVisible();
  // CSS tab is not opened by default
  await expect(cssTabToggle).toHaveAttribute('aria-expanded', 'false');
  await expect(cssTab).not.toBeVisible();
  await cssTabToggle.click();
  await expect(cssTabToggle).toHaveAttribute('aria-expanded', 'true');
  await expect(cssTab).toBeVisible();
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/editor-tabs.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
  );
});

test('should toggle editor visibility based on aria-expanded attribute', async ({
  page
}) => {
  const tabLocator = page.locator('div.monaco-editor-tabs button').first();
  const isExpandedBeforeClick = await tabLocator.getAttribute('aria-expanded');
  if (isExpandedBeforeClick === 'true') {
    await tabLocator.click();
    const isExpandedAfterClick = await tabLocator.getAttribute('aria-expanded');
    expect(isExpandedAfterClick).toBe('false');
  } else {
    await tabLocator.click();
    const isExpandedAfterClick = await tabLocator.getAttribute('aria-expanded');
    expect(isExpandedAfterClick).toBe('true');
  }
});

test('toggling the button should toggle the appearance of the corresponding ReflexElement', async ({
  page
}) => {
  const tabsLocator = await page.locator('div.monaco-editor-tabs button').all();
  for (const tab of tabsLocator) {
    const tabFileKey = await tab.getAttribute('data-cy');
    if (tabFileKey) {
      const fileKey = tabFileKey.replace('editor-tab-', '');

      const ariaExpandedBeforeClick = await tab.getAttribute('aria-expanded');
      const reflexElementBeforeClick = page.locator(
        `div[data-cy="editor-container-${fileKey}"]`
      );


Review comment (suggestion):
We can check the whether the button has `aria-expanded` using:

```ts
await expect(button).toHaveAttribute('aria-expanded', 'true');

await button.click();
await expect(button).toHaveAttribute('aria-expanded', 'false');
```

Code after:
import { test, expect } from '@playwright/test';

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/build-a-survey-form-project/build-a-survey-form'
  );
});

test('should toggle editor visibility correctly', async ({ page }) => {
  const htmlTabToggle = page.getByRole('button', { name: 'index.html Editor' });
  const cssTabToggle = page.getByRole('button', { name: 'styles.css Editor' });
  const htmlTab = page.getByTestId('editor-container-indexhtml');
  const cssTab = page.getByTestId('editor-container-stylescss');

  await expect(htmlTabToggle).toBeVisible();
  // HTML tab is opened by default
  await expect(htmlTabToggle).toHaveAttribute('aria-expanded', 'true');
  await expect(htmlTab).toBeVisible();
  await htmlTabToggle.click();
  await expect(htmlTabToggle).toHaveAttribute('aria-expanded', 'false');
  await expect(htmlTab).not.toBeVisible();

  await expect(cssTabToggle).toBeVisible();
  // CSS tab is not opened by default
  await expect(cssTabToggle).toHaveAttribute('aria-expanded', 'false');
  await expect(cssTab).not.toBeVisible();
  await cssTabToggle.click();
  await expect(cssTabToggle).toHaveAttribute('aria-expanded', 'true');
  await expect(cssTab).toBeVisible();
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/solution-viewer.spec.ts (TypeScript)

Code before:
import { test, expect } from '@playwright/test';

test.use({ storageState: 'playwright/.auth/certified-user.json' });

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/certification/certifieduser/javascript-algorithms-and-data-structures'
  );
});

test.afterEach(async ({ page }) => {
  await page.close();
});

test.describe('Solution Viewer component', () => {
  test('renders the modal correctly', async ({ page }) => {
    await page.getByRole('button').filter({ hasText: /view/i }).first().click();

    // The modal should show the solution title...
    await expect(
      page.getByRole('heading').and(page.getByText(/solution for/i))
    ).toBeVisible();
    // ...and relevant code file/s
    await expect(page.getByText(/js/i)).toBeVisible();
    await expect(page.locator('pre').first()).toBeVisible();
  });
});


Review comment (suggestion):
```suggestion
```

We actually don't need this. Playwright does it automatically.

Code after:
import { test, expect } from '@playwright/test';

test.use({ storageState: 'playwright/.auth/certified-user.json' });

test.beforeEach(async ({ page }) => {
  await page.goto(
    '/certification/certifieduser/javascript-algorithms-and-data-structures'
  );
});

test.describe('Solution Viewer component', () => {
  test('renders the modal correctly', async ({ page }) => {
    await page.getByRole('button').filter({ hasText: /view/i }).first().click();

    // The modal should show the solution title...
    await expect(
      page.getByRole('heading').and(page.getByText(/solution for/i))
    ).toBeVisible();
    // ...and relevant code file/s
    await expect(page.getByText(/js/i)).toBeVisible();
    await expect(page.locator('pre').first()).toBeVisible();
  });
});