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

Set of tests for paginator with entities where id is custom object #7890

Closed
wants to merge 1 commit into from
Closed

Conversation

akorz
Copy link
Contributor

@akorz akorz commented Nov 5, 2019

See more #7837

@Ocramius
Copy link
Member

Ocramius commented Nov 6, 2019

These diffs are still massive - we most certainly need to reduce them to just the minimum viable requirement to understand the failure :S

@akorz
Copy link
Contributor Author

akorz commented Nov 6, 2019

@Ocramius Why? Do you have tests for paginator in your master branch for entities where id is custom object? No. I added them. Along that tests you can see 1 error. That's it. Let's imaging, I added just 1 test. But how can you be sure that all other things with paginator are still ok?

@Ocramius
Copy link
Member

Ocramius commented Nov 6, 2019

I had added them to GH7820 if I remember correctly?

The problem is that a lot of unrelated stuff got committed here, which makes it really hard to understand which detail to look for.

@akorz
Copy link
Contributor Author

akorz commented Nov 6, 2019

Do you mean #7820 is a reason of error? Yes. You tried to create patch #7865 but with no success in my case.

Please check Paginator tests in 2.6 branch. You can see a lot of entities are related with that tests. I created similar entities with custom id. Thats why diff is huge. But if it's necessary for getting things done what else should I do? :)

@akorz
Copy link
Contributor Author

akorz commented Nov 14, 2019

@Ocramius Hello, did you have any chance to check it out? Thanks.

@Ocramius
Copy link
Member

Heya, as mentioned above, the diff is too big to evaluate properly. If you take #7821, for example, I tried isolating everything into GH7820Test

@ostrolucky
Copy link
Member

continued in #7903

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

Successfully merging this pull request may close these issues.

4 participants