Skip to content

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

Closed
akorz wants to merge 1 commit intodoctrine:2.6from
akorz:2.6
Closed

Set of tests for paginator with entities where id is custom object#7890
akorz wants to merge 1 commit intodoctrine:2.6from
akorz:2.6

Conversation

@akorz
Copy link
Copy Markdown
Contributor

@akorz akorz commented Nov 5, 2019

See more #7837

@Ocramius
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

akorz commented Nov 14, 2019

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

@Ocramius
Copy link
Copy Markdown
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
Copy Markdown
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