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

[CHORE] refactor: Remove runloop usage in integration tests for store/ find All module #6712

Conversation

dmuneras
Copy link
Contributor

Description

  • Remove usage of runloop in store - findAll module tests.
  • Remove beforeEach loop usage in the module and setup data in each test.

@dmuneras dmuneras changed the title chore: refactor: Remove runloop usage in integration tests for store/ find All module [chore] : refactor: Remove runloop usage in integration tests for store/ find All module Nov 10, 2019
@dmuneras dmuneras changed the title [chore] : refactor: Remove runloop usage in integration tests for store/ find All module [chore] refactor: Remove runloop usage in integration tests for store/ find All module Nov 10, 2019
@dmuneras dmuneras changed the title [chore] refactor: Remove runloop usage in integration tests for store/ find All module [CHORE] refactor: Remove runloop usage in integration tests for store/ find All module Nov 10, 2019
Copy link
Contributor

@runspired runspired left a comment

Choose a reason for hiding this comment

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

Overall this is good but I don't think we should introduce the "escape the microtask queue" trick for tests that don't need it.

e.g. this

await new Promise(resolve => setTimeout(resolve, 1));

lets refactor these to just return resolve( ...payload... ) vs the async/await with the timeout

packages/-ember-data/tests/integration/store-test.js Outdated Show resolved Hide resolved
packages/-ember-data/tests/integration/store-test.js Outdated Show resolved Hide resolved
@runspired runspired added 🏷️ test This PR primarily adds tests for a feature 🎯 canary PR is targeting canary (default) 🔌 Project Unplug 🔌 labels Nov 11, 2019
@dmuneras dmuneras force-pushed the refactor/find-all-model-in-store-integration-test branch from 5e6f887 to dd756c7 Compare November 12, 2019 18:17
@runspired runspired merged commit 14b38df into emberjs:master Nov 13, 2019
@runspired runspired removed the 🎯 canary PR is targeting canary (default) label May 27, 2021
@runspired runspired added this to the 🔌 Unplug milestone Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ test This PR primarily adds tests for a feature 🔌 Project Unplug 🔌
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

2 participants