Skip to content

Comments

[8.x] Cleanup tests#39535

Merged
taylorotwell merged 6 commits into8.xfrom
cleanup-tests
Nov 8, 2021
Merged

[8.x] Cleanup tests#39535
taylorotwell merged 6 commits into8.xfrom
cleanup-tests

Conversation

@driesvints
Copy link
Member

@driesvints driesvints commented Nov 8, 2021

This PR contains the following changes:

  • Do not explicitly set debug mode anymore. These calls were probably legacy code lingering.
  • Move MySQL specific tests in a separate MySql directory so they're grouped per-engine and easier discoverable. This way we can remove the @group call as well.
  • Modify EloquentUpdateTest & EloquentDeleteTest to run on all engines and skip a few tests on MSSQL.

@driesvints driesvints changed the title cleanup tests [8.x] Cleanup tests Nov 8, 2021

Post::join('comments', 'comments.post_id', '=', 'posts.id')->where('posts.id', '>', 1)->orderBy('posts.id')->limit(1)->delete();
Post::join('comments', 'comments.post_id', '=', 'posts.id')
->where('posts.id', '>', 8)
Copy link
Member Author

Choose a reason for hiding this comment

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

SQLite and MySQL behave different here so we should adjust the test accordingly.


if ($this->driver !== 'mysql') {
$this->markTestSkipped('Test requires a MySQL connection.');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Skip tests on non-MySQL connections so they don't error out when using other drivers.

@driesvints driesvints marked this pull request as ready for review November 8, 2021 17:37
Copy link
Collaborator

@GrahamCampbell GrahamCampbell left a comment

Choose a reason for hiding this comment

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

:shipit:

@taylorotwell taylorotwell merged commit 2ecbb45 into 8.x Nov 8, 2021
@taylorotwell taylorotwell deleted the cleanup-tests branch November 8, 2021 20:30
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.

3 participants