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

[9.x] Reset refresh database state between tests #198

Merged
merged 1 commit into from Mar 19, 2024

Conversation

crynobone
Copy link
Member

No description provided.

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
@crynobone crynobone marked this pull request as ready for review March 19, 2024 00:57
@crynobone crynobone added the bug label Mar 19, 2024
@crynobone crynobone changed the title Reset refresh database state between tests [9.x] Reset refresh database state between tests Mar 19, 2024
@coveralls
Copy link

Coverage Status

coverage: 92.414% (+0.01%) from 92.402%
when pulling 3a575f7 on 9/reset-refresh-database-state-between-tests
into c86b66e on 9.x.

@crynobone crynobone merged commit dcb041e into 9.x Mar 19, 2024
78 checks passed
@crynobone crynobone deleted the 9/reset-refresh-database-state-between-tests branch March 19, 2024 01:06
@brunogaspar
Copy link

@crynobone Hello 👋

Sadly, this has caused a regression, at least on my end. I'm also aware this logic was moved to the InteractsWithTestCase trait.

I've been using the RefreshDatabase trait for quite some time, even after Laravel 11 upgrade, but after this change, the trait still works, but the migrations are executed before each test, which seems unwanted (at least from my point of view), which in turn makes the tests slow as the more migrations you have, the slow it will be 😞

Do i need to change something on my end?

Thank you!!

@crynobone

This comment was marked as outdated.

@crynobone
Copy link
Member Author

See 8930529

@crynobone
Copy link
Member Author

Oh yeah, my mistake. There are two RefreshDatabase usages in Testbench.

  1. Consistent migrations per test case.
  2. Ad-hoc migrations between tests in test cases.

While Laravel application mostly uses structure 1, Package developers have been known to use structure 2, which led to the above changes. We will need to rethink this

@brunogaspar
Copy link

No problem.

I'm pretty much on structure 1 but on a package, testing against a MySQL database not a in memory one (i know it's a bit unconventional).

I have for now implemented a workaround so it does not use the RefreshDatabaseState::$migrated

Let me know if there's anything i can help to address it.

@crynobone
Copy link
Member Author

crynobone commented Mar 24, 2024

I'm pretty much on structure 1 but on a package, testing against a MySQL database not a in memory one (i know it's a bit unconventional).

RefreshDatabase in Laravel 11 for :in-memory: was changed to work similarly to a persistent database which prompted these changes (tests will break with structure 2).

@crynobone
Copy link
Member Author

@brunogaspar can you test the latest patch release and see if that fixed your issue.

@brunogaspar
Copy link

@crynobone Yes, that seemed to have fixed it.

Thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants