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.

Sign up for GitHub

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

Jump to bottom

[CHORE] Modernize integration/ tests and remove run loop calls #6221

Merged
merged 1 commit into from
Aug 20, 2019
Merged

[CHORE] Modernize integration/ tests and remove run loop calls #6221

merged 1 commit into from
Aug 20, 2019

Conversation

pete-the-pete
Copy link
Contributor

Updating a few tests at the top level of the integration directory...more to come but wanted to get something up to make sure i'm on the right track.

@runspired

@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch 2 times, most recently from 1cbc44d to b393b1b Compare July 8, 2019 14:11
@pete-the-pete
Copy link
Contributor Author

@runspired the integration/record_array_manager failing tests are passing locally...any ideas?

@runspired
Copy link
Contributor

I'll pull down and see if I can spot why they seem to pass in one place and not the other, could be a good pairing session. @pete-the-pete

runspired
runspired requested changes Jul 12, 2019
View reviewed changes
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.

Looks great! couple of nits. Will pull down to check locally why this might be failing.

packages/-ember-data/tests/integration/inverse-test.js Outdated Show resolved Hide resolved
packages/-ember-data/tests/integration/inverse-test.js Outdated Show resolved Hide resolved
@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch 4 times, most recently from e32413e to f80b5ec Compare July 16, 2019 18:07
runspired
runspired reviewed Jul 17, 2019
View reviewed changes
packages/-ember-data/package.json Outdated Show resolved Hide resolved
packages/-ember-data/package.json Outdated Show resolved Hide resolved
packages/-ember-data/tests/integration/inverse-test.js Outdated Show resolved Hide resolved
@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch 3 times, most recently from 56ce4b7 to bba3f07 Compare July 23, 2019 15:50
@snewcomer
Copy link
Contributor

@pete-the-pete Any chance we can revisit this PR (perhaps the test run is cleaned up and will pass)? I'm about to start work on the Adapter changes from #6166 and want to make sure we both are complementing each other's work!

@pete-the-pete
Copy link
Contributor Author

@snewcomer i'll rebase and see.

@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch from bba3f07 to 492357a Compare August 19, 2019 18:15
@pete-the-pete
Copy link
Contributor Author

Looks like maybe some failures with partner tests...i can take a look in the new couple days.

@rwjblue Octobox
Copy link
Member

rwjblue commented Aug 20, 2019

I think you should rebase, @igor and @herioiceric (amongst others) recently got CI passing again (including tweaks / mitigations for partner tests). Rebasing should get you using those changes, so we can see if there are still any failures...

@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch from 492357a to 81cc67b Compare August 20, 2019 18:42
@runspired runspired added the 🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166 label Aug 20, 2019
runspired
runspired approved these changes Aug 20, 2019
View reviewed changes
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.

One minor nit remaining but 👍

packages/-ember-data/tests/integration/inverse-test.js Outdated Show resolved Hide resolved
@runspired
Copy link
Contributor

We can ignore the Travis partner test error, it is a known flakey test and more recent CI runs have passed.

@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch from 81cc67b to e34475a Compare August 20, 2019 20:59
@pete-the-pete
Copy link
Contributor Author

welp....lots more failures 😢 , i'll look.

@pete-the-pete pete-the-pete force-pushed the chore-modernize-integration-tests branch from e34475a to 3119c09 Compare August 20, 2019 22:55
@runspired runspired merged commit 5b1f559 into emberjs:master Aug 20, 2019
@runspired runspired mentioned this pull request Aug 23, 2019
Closed
65 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@runspired runspired runspired approved these changes

Assignees
No one assigned
Labels
🌲 Project Trim 🌲 PRs related to https://github.com/emberjs/data/issues/6166
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pete-the-pete @runspired @snewcomer @rwjblue