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

remove eager_load from test env #2216

Closed
wants to merge 1 commit into from

Conversation

dm1try
Copy link
Member

@dm1try dm1try commented Dec 29, 2021

as we have a separated spec for this case

partially reverts 752bf4c

ref #2214

@grape-bot

This comment has been minimized.

@dm1try dm1try requested review from dnesteryuk and dblock December 29, 2021 14:18
@dm1try
Copy link
Member Author

dm1try commented Dec 29, 2021

Would love an explanation of why this fixes that and a spec that shows that it does, please?

it does not fix anything it just shows that the lib does not load some files when eager_load is disabled(what's usually the case in dev environment)
it is also shows that a separated spec has a false positive result, as we eager load the whole lib code in the spec helper.

so, the pr is just the reply for your #2214 (comment) , the tests are red :)

as we have a separated spec for this case
@dm1try dm1try force-pushed the ensure_we_test_lazy_loading branch from f50c99b to 5dc9408 Compare December 30, 2021 17:57
@dblock
Copy link
Member

dblock commented Jan 3, 2022

Closed via #2222

@dblock dblock closed this Jan 3, 2022
@dm1try dm1try deleted the ensure_we_test_lazy_loading branch January 3, 2022 13:54
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