Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hotfix: e2e tests #2164

Merged
merged 2 commits into from
Mar 1, 2024
Merged

hotfix: e2e tests #2164

merged 2 commits into from
Mar 1, 2024

Conversation

pandeymangg
Copy link
Contributor

What does this PR do?

Fixes # (issue)

How should this be tested?

  • Test A
  • Test B

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read How we Code at Formbricks
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Formbricks Docs if changes were necessary

Copy link

vercel bot commented Mar 1, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
formbricks-cloud ⬜️ Ignored (Inspect) Mar 1, 2024 10:32am
formbricks-com ⬜️ Ignored (Inspect) Mar 1, 2024 10:32am

Copy link
Contributor

github-actions bot commented Mar 1, 2024

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

Unknown release type "hotfix" found in pull request title "hotfix: e2e tests". 

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

Copy link
Contributor

apps/web/playwright/utils/helper.ts

It's a good practice to use constants for repeated strings. This makes the code easier to maintain and less prone to typos.
Create Issue
See the diff
Checkout the fix

    const EMAIL_PLACEHOLDER = "work@email.com";
    const PASSWORD_PLACEHOLDER = "*******";
    const CONTINUE_WITH_EMAIL_BUTTON = "Continue with Email";
    const LOGIN_WITH_EMAIL_BUTTON = "Login with Email";
    const IN_APP_SURVEYS_BUTTON = "In-app Surveys Run a survey";
    const I_AM_NOT_SURE_BUTTON = "I am not sure how to do this";
    const INVITE_BUTTON = "Invite";
    const MY_PRODUCT_TEXT = "My Product";
git fetch origin && git checkout -b ReviewBot/Impro-cbmbfvj origin/ReviewBot/Impro-cbmbfvj

apps/web/playwright/js.spec.ts

It's a good practice to minimize the use of waitForTimeout as it can slow down the test execution. Instead, use more specific wait conditions like waitForSelector, waitForResponse, waitForNavigation, etc. This makes the tests more reliable and faster.
Create Issue
See the diff
Checkout the fix

    // Replace
    await page.waitForTimeout(1000);
    // With
    await page.waitForSelector("selector");
git fetch origin && git checkout -b ReviewBot/Impro-22sf6ld origin/ReviewBot/Impro-22sf6ld

apps/web/app/(app)/environments/[environmentId]/settings/members/components/EditMemberships/MemberActions.tsx

It's a good practice to avoid logging errors directly to the console in production code. This can potentially expose sensitive information. Instead, use a logging library that can be configured to log errors in a secure manner.
Create Issue
See the diff
Checkout the fix

    // Replace
    console.log({ err });
    // With
    logger.error(err);
git fetch origin && git checkout -b ReviewBot/Impro-2by7rqk origin/ReviewBot/Impro-2by7rqk

@@ -64,7 +64,8 @@ test.describe("JS Package Test", async () => {
// Formbricks Modal is visible
await expect(page.getByRole("link", { name: "Powered by Formbricks" })).toBeVisible();

await page.waitForTimeout(1000);
await page.waitForLoadState("networkidle");
await page.waitForTimeout(1500);
Copy link
Contributor

Choose a reason for hiding this comment

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

Replacing waitForTimeout with more specific wait conditions to improve test performance and reliability.

Suggested change
await page.waitForTimeout(1500);
await page.waitForSelector("#formbricks-modal-container");

@pandeymangg pandeymangg added this pull request to the merge queue Mar 1, 2024
Merged via the queue into main with commit 9fdb745 Mar 1, 2024
13 of 16 checks passed
@pandeymangg pandeymangg deleted the hotfix/e2e-tests branch March 1, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant