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

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

const outputTexts = {
  default: `
  /**
  * Your test output will go here
  */`,
  syntaxError: `SyntaxError: unknown: Unexpected token (1:3)

  > 1 | var
      |    ^`,
  empty: `// running tests
  You should declare myName with the var keyword, ending with a semicolon
  // tests completed`,
  passed: `// running tests
// tests completed`
};

const insertTextInCodeEditor = async (
  page: Page,
  isMobile: boolean,
  text: string
) => {
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].code })
      .click();
  }
  await page.getByLabel('Editor content').fill(text);
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].console })
      .click();
  }
};

const runChallengeTest = async (page: Page, isMobile: boolean) => {
  if (isMobile) {
    await page.getByRole('tab', { name: 'Code' }).click();
    await page.getByText('Run').click();
    await page.getByRole('tab', { name: 'Console' }).click();
  } else {
    await page.getByText('Run the Tests (Ctrl + Enter)').click();
  }
};

test.beforeEach(async ({ page }) => {
  await page.goto(

Review comment (suggestion):
Let's change the function signature to taking an object instead of 3 parameters. It would improve the readability a bit as we won't have to memorize the param order and param type.

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

const outputTexts = {
  default: `
  /**
  * Your test output will go here
  */`,
  syntaxError: `SyntaxError: unknown: Unexpected token (1:3)

  > 1 | var
      |    ^`,
  empty: `// running tests
  You should declare myName with the var keyword, ending with a semicolon
  // tests completed`,
  passed: `// running tests
// tests completed`
};

interface InsertTextParameters {
  page: Page;
  isMobile: boolean;
  text: string;
}

const insertTextInCodeEditor = async ({
  page,
  isMobile,
  text
}: InsertTextParameters) => {
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].code })
      .click();
  }
  await page.getByLabel('Editor content').fill(text);
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].console })
      .click();
  }
};

const runChallengeTest = async (page: Page, isMobile: boolean) => {
  if (isMobile) {
    await page.getByRole('tab', { name: 'Code' }).click();
    await page.getByText('Run').click();
    await page.getByRole('tab', { name: 'Console' }).click();
  } else {
Repository: freeCodeCamp/freeCodeCamp
File: e2e/output.spec.ts (TypeScript)

Code before:

  test('should contain reference error output when var is entered in editor', async ({
    page,
    isMobile
  }) => {
    const referenceErrorRegex =
      /ReferenceError: (myName is not defined|Can't find variable: myName)/;
    await insertTextInCodeEditor(page, isMobile, 'myName');
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(referenceErrorRegex);
  });
  test('should contain final output after test fail', async ({
    page,
    isMobile
  }) => {
    await runChallengeTest(page, isMobile);
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(outputTexts.empty);
  });
  test('should contain final output after test pass', async ({
    page,
    isMobile
  }) => {
    const closeButton = page.getByRole('button', { name: 'Close' });

    await insertTextInCodeEditor(page, isMobile, 'var myName;');
    await runChallengeTest(page, isMobile);
    await closeButton.click();
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(outputTexts.passed);
  });
});


Review comment (suggestion):
```suggestion
  });
  
  test('should contain final output after test pass', async ({
```

Code after:
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(outputTexts.syntaxError);
  });

  test('should contain reference error output when var is entered in editor', async ({
    page,
    isMobile
  }) => {
    const referenceErrorRegex =
      /ReferenceError: (myName is not defined|Can't find variable: myName)/;
    await insertTextInCodeEditor({ page, isMobile, text: 'myName' });
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(referenceErrorRegex);
  });

  test('should contain final output after test fail', async ({
    page,
    isMobile
  }) => {
    await runChallengeTest(page, isMobile);
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(outputTexts.empty);
  });

  test('should contain final output after test pass', async ({
    page,
    isMobile
  }) => {
    const closeButton = page.getByRole('button', { name: 'Close' });

    await insertTextInCodeEditor({ page, isMobile, text: 'var myName;' });
    await runChallengeTest(page, isMobile);
    await closeButton.click();
    await expect(
      page.getByRole('region', {
        name: translations.learn['editor-tabs'].console
      })
    ).toHaveText(outputTexts.passed);
  });
});
Repository: freeCodeCamp/freeCodeCamp
File: e2e/output.spec.ts (TypeScript)

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

const outputTexts = {
  default: `
  /**
  * Your test output will go here
  */`,
  syntaxError: `SyntaxError: unknown: Unexpected token (1:3)

  > 1 | var
      |    ^`,
  empty: `// running tests
  You should declare myName with the var keyword, ending with a semicolon
  // tests completed`,
  passed: `// running tests
// tests completed`
};
interface InsertTextParameters {
  page: Page;
  isMobile: boolean;
  text: string;
}
const insertTextInCodeEditor = async (paras: InsertTextParameters) => {
  if (paras.isMobile) {
    await paras.page
      .getByRole('tab', { name: translations.learn['editor-tabs'].code })
      .click();
  }
  await paras.page.getByLabel('Editor content').fill(paras.text);
  if (paras.isMobile) {
    await paras.page
      .getByRole('tab', { name: translations.learn['editor-tabs'].console })
      .click();
  }
};

const runChallengeTest = async (page: Page, isMobile: boolean) => {
  if (isMobile) {
    await page.getByRole('tab', { name: 'Code' }).click();
    await page.getByText('Run').click();
    await page.getByRole('tab', { name: 'Console' }).click();
  } else {
    await page.getByText('Run the Tests (Ctrl + Enter)').click();
  }

Review comment (suggestion):
Let's restore the empty line for readability:

```suggestion

interface InsertTextParameters {
```

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

const outputTexts = {
  default: `
  /**
  * Your test output will go here
  */`,
  syntaxError: `SyntaxError: unknown: Unexpected token (1:3)

  > 1 | var
      |    ^`,
  empty: `// running tests
  You should declare myName with the var keyword, ending with a semicolon
  // tests completed`,
  passed: `// running tests
// tests completed`
};

interface InsertTextParameters {
  page: Page;
  isMobile: boolean;
  text: string;
}

const insertTextInCodeEditor = async ({
  page,
  isMobile,
  text
}: InsertTextParameters) => {
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].code })
      .click();
  }
  await page.getByLabel('Editor content').fill(text);
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].console })
      .click();
  }
};

const runChallengeTest = async (page: Page, isMobile: boolean) => {
  if (isMobile) {
Repository: freeCodeCamp/freeCodeCamp
File: e2e/output.spec.ts (TypeScript)

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

const outputTexts = {
  default: `
  /**
  * Your test output will go here
  */`,
  syntaxError: `SyntaxError: unknown: Unexpected token (1:3)

  > 1 | var
      |    ^`,
  empty: `// running tests
  You should declare myName with the var keyword, ending with a semicolon
  // tests completed`,
  passed: `// running tests
// tests completed`
};
interface InsertTextParameters {
  page: Page;
  isMobile: boolean;
  text: string;
}
const insertTextInCodeEditor = async (paras: InsertTextParameters) => {
  if (paras.isMobile) {
    await paras.page
      .getByRole('tab', { name: translations.learn['editor-tabs'].code })
      .click();
  }
  await paras.page.getByLabel('Editor content').fill(paras.text);
  if (paras.isMobile) {
    await paras.page
      .getByRole('tab', { name: translations.learn['editor-tabs'].console })
      .click();
  }
};

const runChallengeTest = async (page: Page, isMobile: boolean) => {
  if (isMobile) {
    await page.getByRole('tab', { name: 'Code' }).click();
    await page.getByText('Run').click();
    await page.getByRole('tab', { name: 'Console' }).click();
  } else {
    await page.getByText('Run the Tests (Ctrl + Enter)').click();
  }
};

test.beforeEach(async ({ page }) => {
  await page.goto(

Review comment (suggestion):
Nit: We could destructure the object here

```suggestion
const insertTextInCodeEditor = async ({ page, isMobile, text }: InsertTextParameters) => {
```

which allows us to keep referencing the properties without having to use `paras`.

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

const outputTexts = {
  default: `
  /**
  * Your test output will go here
  */`,
  syntaxError: `SyntaxError: unknown: Unexpected token (1:3)

  > 1 | var
      |    ^`,
  empty: `// running tests
  You should declare myName with the var keyword, ending with a semicolon
  // tests completed`,
  passed: `// running tests
// tests completed`
};

interface InsertTextParameters {
  page: Page;
  isMobile: boolean;
  text: string;
}

const insertTextInCodeEditor = async ({
  page,
  isMobile,
  text
}: InsertTextParameters) => {
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].code })
      .click();
  }
  await page.getByLabel('Editor content').fill(text);
  if (isMobile) {
    await page
      .getByRole('tab', { name: translations.learn['editor-tabs'].console })
      .click();
  }
};

const runChallengeTest = async (page: Page, isMobile: boolean) => {
  if (isMobile) {
    await page.getByRole('tab', { name: 'Code' }).click();
    await page.getByText('Run').click();
    await page.getByRole('tab', { name: 'Console' }).click();
  } else {
Repository: freeCodeCamp/freeCodeCamp
File: e2e/reset-modal.spec.ts (TypeScript)

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

import translations from '../client/i18n/locales/english/translations.json';

test.beforeEach(({ browserName }) => {
  test.skip(
    browserName === 'webkit',
    'Failing on webkit for no apparent reason. Can not reproduce locally.'
  );
});

test('Check the rendering of the reset modal', async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/learn-html-by-building-a-cat-photo-app/step-2'
  );

  await page.getByTestId('lowerJaw-reset-button').click();
  await expect(page.getByTestId('reset-modal')).toBeVisible();
  await expect(
    page.getByRole('button', {
      name: translations.buttons.close
    })
  ).toBeVisible();
  const modalTitle = page.getByRole('heading', {
    name: 'Reset this lesson?'
  });
  await expect(modalTitle).toBeVisible();

  await expect(page.getByText(translations.learn['reset-warn'])).toBeVisible();
  await expect(
    page.getByText(translations.learn['reset-warn-2'])
  ).toBeVisible();

  await expect(
    page.getByRole('button', {
      name: translations.buttons['reset-lesson']
    })

Review comment (suggestion):
```suggestion
test('should render the modal content correctly', async ({ page }) => {
```

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

import translations from '../client/i18n/locales/english/translations.json';

test.beforeEach(({ browserName }) => {
  test.skip(
    browserName === 'webkit',
    'Failing on webkit for no apparent reason. Can not reproduce locally.'
  );
});

test('should render the modal content correctly', async ({ page }) => {
  await page.goto(
    '/learn/2022/responsive-web-design/learn-html-by-building-a-cat-photo-app/step-2'
  );

  await page.getByRole('button', { name: translations.buttons.reset }).click();

  // There are two elements with the `dialog` role in the DOM.
  // This appears to be semantically incorrect and should be resolved
  // once we have migrated the component to use Dialog from the `ui-components` library.
  const dialogs = await page.getByRole('dialog').all();
  expect(dialogs).toHaveLength(2);

  await expect(
    page.getByRole('button', {
      name: translations.buttons.close
    })
  ).toBeVisible();
  await expect(
    page.getByRole('heading', {
      name: translations.learn.reset
    })
  ).toBeVisible();

  await expect(page.getByText(translations.learn['reset-warn'])).toBeVisible();
  await expect(
Repository: freeCodeCamp/freeCodeCamp
File: e2e/footer.spec.ts (TypeScript)

Code before:

test.describe('Footer Trending Guides section', () => {
  test('should render the section with a header and 30 articles', async ({
    page
  }) => {
    await expect(
      page.getByRole('heading', {
        name: translations.footer['trending-guides']
      })
    ).toBeVisible();

    const articles = await page
      .getByRole('list', { name: translations.footer['trending-guides'] })
      .getByRole('link')
      .all();

    expect(articles).toHaveLength(30);

    for (const article of articles) {
      await expect(article).toBeVisible();
    }
  });
});

test.describe('Footer mobile app section', () => {
  test('should render the download link correctly', async ({ page }) => {
    await expect(
      page.getByRole('heading', {
        level: 2,
        name: translations.footer['mobile-app']
      })
    ).toBeVisible();

    const downloadLinks = await page
      .getByRole('list', { name: translations.footer['mobile-app'] })
      .getByRole('listitem')
      .all();

    expect(downloadLinks).toHaveLength(2);

    const appleStoreLink = downloadLinks[0];

    await expect(
      appleStoreLink.getByRole('img', { name: 'Apple Store' })
    ).toBeVisible();
    await expect(
      appleStoreLink.getByRole('link', { name: 'Apple Store' })
    ).toBeVisible();
    await expect(
      appleStoreLink.getByRole('link', { name: 'Apple Store' })
    ).toHaveAttribute(

Review comment (suggestion):
```suggestion
  test('should render the download links correctly', async ({ page }) => {
```

Code after:

test.describe('Footer Trending Guides section', () => {
  test('should render the section with a header and 30 articles', async ({
    page
  }) => {
    await expect(
      page.getByRole('heading', {
        name: translations.footer['trending-guides']
      })
    ).toBeVisible();

    const articles = await page
      .getByRole('list', { name: translations.footer['trending-guides'] })
      .getByRole('link')
      .all();

    expect(articles).toHaveLength(30);

    for (const article of articles) {
      await expect(article).toBeVisible();
    }
  });
});

test.describe('Footer mobile app section', () => {
  test('should render the download links correctly', async ({ page }) => {
    await expect(
      page.getByRole('heading', {
        level: 2,
        name: translations.footer['mobile-app']
      })
    ).toBeVisible();

    const downloadLinks = await page
      .getByRole('list', { name: translations.footer['mobile-app'] })
      .getByRole('listitem')
      .all();

    expect(downloadLinks).toHaveLength(2);

    const appleStoreLink = downloadLinks[0];

    await expect(
      appleStoreLink.getByRole('img', { name: 'Apple Store' })
    ).toBeVisible();
    await expect(
      appleStoreLink.getByRole('link', { name: 'Apple Store' })
    ).toBeVisible();
    await expect(
      appleStoreLink.getByRole('link', { name: 'Apple Store' })
    ).toHaveAttribute(
Repository: freeCodeCamp/freeCodeCamp
File: e2e/show.spec.ts (TypeScript)

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

// Tests for challenges rendered by `client/src/templates/Challenges/odin/show.tsx`
test.describe('Odin challenges', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(
      '/learn/foundational-c-sharp-with-microsoft/write-your-first-code-using-c-sharp/write-your-first-c-sharp-code'
    );
  });

  test.describe('When the user is signed out', () => {
    test.use({ storageState: { cookies: [], origins: [] } });

    test('should render the content correctly', async ({ page }) => {
      await expect(page).toHaveTitle(
        'Write Your First Code Using C# - Write Your First C# Code | Learn | freeCodeCamp.org'
      );

      await expect(
        page.getByRole('heading', {
          level: 1,
          name: 'Write Your First C# Code'
        })
      ).toBeVisible();

      // Checkmark doesn't show up if the user has completed the challenge but is signed out
      await expect(
        page.getByRole('img', {
          name: translations.icons.passed,

Review comment (suggestion):
Since we have multiple `show.tsx` files and each has different rendering logic, I think we should have a dedicated `show.spec.ts` for each of them. This file happens to test against a C# challenge, so I reused it and named the test suite "Odin challenges" to be a bit more explicit.

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

// Tests for challenges rendered by `client/src/templates/Challenges/odin/show.tsx`
test.describe('Odin challenges', () => {
  test.beforeEach(async ({ page }) => {
    await page.goto(
      '/learn/foundational-c-sharp-with-microsoft/write-your-first-code-using-c-sharp/write-your-first-c-sharp-code'
    );
  });

  test.describe('When the user is signed out', () => {
    test.use({ storageState: { cookies: [], origins: [] } });

    test('should render the content correctly', async ({ page }) => {
      await expect(page).toHaveTitle(
        'Write Your First Code Using C# - Write Your First C# Code | Learn | freeCodeCamp.org'
      );

      await expect(
        page.getByRole('heading', {
          level: 1,
          name: 'Write Your First C# Code'
        })
      ).toBeVisible();

      // Checkmark doesn't show up if the user has completed the challenge but is signed out
      await expect(
        page.getByRole('img', { name: translations.icons.passed })
      ).toBeHidden();
Repository: freeCodeCamp/freeCodeCamp
File: e2e/update-email.spec.ts (TypeScript)

Code before:
  test('The page has update email form', async () => {
    const form = page.getByTestId('update-email-form');
    const emailInput = page.getByTestId('update-email-input');
    const submitButton = page.getByTestId('update-email-submit-button');

    await expect(form).toBeVisible();
    await expect(emailInput).toHaveAttribute('type', 'email');
    await expect(emailInput).toHaveAttribute(
      'placeholder',
      'camperbot@example.com'
    );
    await expect(submitButton).toHaveAttribute('type', 'submit');
    await expect(submitButton).toContainText(
      translations.buttons['update-email']
    );
  });

  test('The page has sign out button', async () => {
    const signOutButton = page.getByTestId('update-email-sign-out-button');

    await expect(signOutButton).toBeVisible();
    await expect(signOutButton).toContainText(translations.buttons['sign-out']);
    await expect(signOutButton).toHaveAttribute('href', '/signout');
  });

  test('input box should function correctly', async () => {
    const emailInput = page.getByTestId('update-email-input');
    const submitButton = page.getByTestId('update-email-submit-button');
    await expect(submitButton).toBeDisabled();
    await emailInput.fill('123');
    await expect(submitButton).toBeDisabled();
    await emailInput.fill('123@gmail.com');
    await expect(submitButton).toBeEnabled();
  });
});


Review comment (suggestion):
```suggestion
  test('should enable the submit button if the email input is not empty', async () => {
```

Code after:
  test('The page has update email form', async () => {
    const form = page.getByTestId('update-email-form');
    const emailInput = page.getByLabel(translations.misc.email);
    const submitButton = page.getByRole('button', {
      name: translations.buttons['update-email']
    });

    await expect(form).toBeVisible();
    await expect(emailInput).toBeVisible();
    await expect(emailInput).toHaveAttribute('type', 'email');
    await expect(emailInput).toHaveAttribute(
      'placeholder',
      'camperbot@example.com'
    );
    await expect(submitButton).toBeVisible();
    await expect(submitButton).toHaveAttribute('type', 'submit');
  });

  test('The page has sign out button', async () => {
    const signOutButton = page.getByRole('link', {
      name: translations.buttons['sign-out']
    });

    await expect(signOutButton).toBeVisible();
    await expect(signOutButton).toHaveAttribute('href', '/signout');
  });

  test('should enable the submit button if the email input is valid', async () => {
    const emailInput = page.getByLabel(translations.misc.email);
    const submitButton = page.getByRole('button', {
      name: translations.buttons['update-email']
    });
    await expect(submitButton).toBeDisabled();
    await emailInput.fill('123');
    await expect(submitButton).toBeDisabled();
    await emailInput.fill('123@gmail.com');
    await expect(submitButton).toBeEnabled();
  });
});