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

Apply limit to database rather than collection #30144

Closed
wants to merge 1 commit into from
Closed

Apply limit to database rather than collection #30144

wants to merge 1 commit into from

Conversation

morrislaptop
Copy link
Contributor

If the following conditions are true:

  • The test is using DatabaseTransactions rather than RefreshDatabase;
  • There is a large dataset for the table under test;
  • The assertDatabaseHas assertion is being used; and
  • The assertion fails

The entire table is put into memory causing allowed memory fatal errors.

This is because the limit is applied on a Collection after putting all the records in memory.

This change puts the limit on the database query so only the limited amount of records go into memory.

This avoids errors in tests for users and does not break any existing features.

@driesvints
Copy link
Member

This seems to break the test suite. Feel free to resubmit once you've fixed the tests.

@morrislaptop
Copy link
Contributor Author

@driesvints done with #30148, thanks!

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.

2 participants