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

Fix E2E test issues with duplicate projects #1428

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

rmunn
Copy link
Collaborator

@rmunn rmunn commented Jun 23, 2022

Description

A couple different things were causing duplicate E2E projects to be created with the same projectCode:

  • Duplicate project-settings.spec.ts and projects-settings.spec.ts files
  • An import { Project } from 'projects-settings' line
  • deleteOne() instead of deleteMany() in TestControl.php

The import line was causing the project settings file to be re-executed, including its test.describe() calls, so that its beforeAll() was getting called twice. By moving the definition of that type into a separate file, we avoid that issue. And by removing the duplicate test file, we avoid the other place where duplicate projects with the same project code were being created.

Type of Change

Only keep lines below that describe this change, then delete the rest.

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Please provide screenshots / animations for any change that involves the UI. Please provide animations to demonstrate user interaction / behavior changes

Testing on your branch

Please describe how to test and/or verify your changes. Provide instructions so we can reproduce. Please also provide relevant test data as necessary. These instructions will be used for QA testing below.

  • Run make; make playwright-tests

Checklist

  • I have performed a self-review of my own code
  • I have reviewed the title/description of this PR which will be used as the squashed PR commit message
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

qa.languageforge.org testing

Reviewers: add/replace your name below and check the box to sign-off/attest the feature works as expected on qa.languageforge.org

  • Reviewer1 (YYYY-MM-DD HH:MM)
  • Reviewer2 (YYYY-MM-DD HH:MM)

@rmunn rmunn requested a review from JeanneSon June 23, 2022 06:09
@github-actions
Copy link

github-actions bot commented Jun 23, 2022

Unit Test Results

373 tests   373 ✔️  11s ⏱️
    1 suites      0 💤
    1 files        0

Results for commit 0741f0e.

♻️ This comment has been updated with latest results.

@rmunn rmunn force-pushed the chore/fix-e2e-test-duplicate-projects branch from 468e210 to a531c2a Compare June 23, 2022 14:11
@rmunn rmunn enabled auto-merge (squash) June 23, 2022 14:12
A couple different things were causing duplicate E2E projects to be
created with the same projectCode:

* Duplicate project-settings.spec.ts and projects-settings.spec.ts files
* An `import { Project } from 'projects-settings'` line
* deleteOne() instead of deleteMany() in TestControl.php

The `import` line was causing the project settings file to be
re-executed, including its `test.describe()` calls, so that its
beforeAll() was getting called twice. By moving the definition of that
type into a separate file, we avoid that issue. And by removing the
duplicate test file, we avoid the other place where duplicate projects
with the same project code were being created.

In addition, another test was failing due to a timeout if the LD API
server wasn't running. Added the `ld-api` container as a dependency of
the Playwright E2E test container to address this issue.
@rmunn rmunn force-pushed the chore/fix-e2e-test-duplicate-projects branch from a531c2a to 0741f0e Compare June 23, 2022 14:40
@rmunn rmunn merged commit eb26671 into develop Jun 23, 2022
@rmunn rmunn deleted the chore/fix-e2e-test-duplicate-projects branch June 23, 2022 15:02
@longrunningprocess longrunningprocess self-assigned this Jun 23, 2022
@longrunningprocess longrunningprocess added the testing Pertaining to unit or e2e testing label Jun 23, 2022
@rmunn rmunn removed the request for review from JeanneSon June 23, 2022 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Pertaining to unit or e2e testing
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants