test: Add more ACP integration tests

[islamaliev]

Resolves #2474 and #2475

Add integration tests for ACP on relation objects and _avg and _count methods.

[AndrewSisley]

Please break up the functions so that there is one per test. And please make it a little easier for readers to see where the results come from.

[AndrewSisley]

thought: I cannot see what this test is really testing, unless I bounce between multiple files trying to hold different bits of info in my head and do the maths myself.

suggestion: It would be much easier to read if the stuff under test was all in the same place, and there we comments in useful places (e.g. just above the _avg results field explaining where the number came from).

[islamaliev]

I cannot see what this test is really testing, unless I bounce between multiple files trying to hold different bits of info in my head and do the maths myself.

This confrontation will never end :)
I agree with your point that the tests needs to be broken up into smaller bits. But then the only thing that will make them different is a single request. And if I inline the setup part it will take 90% of the whole test body.

There is no need to "bounce between multiple files", it's one just to the setup method (potentially only once for all tests) and back.

Phrases like "It would be much easier to read" are highly subjective as it expresses your personal perception.
For me personally it would take much more time to read tests with "the stuff all in the same place", because all this 90% of copy&pasted code needs to be checked as well, right? Otherwise why is it there? It's good when you know the setup code is the same in all tests, but what if you don't?
With the current approach you know it immediately and you see right away what makes these tests differ from one another.

[AndrewSisley]

This confrontation will never end :)

I'll resist responding here then, as it will just bog the PR down :P Maybe worth a discord thread after the release though.

Please document the tests as-is then, a few code comments explaining what is going on to anyone reading can still greatly improve things here.

[islamaliev]

I added some comments when I though it would benefit a reader. Let me know if you think any other part needs more comments.

[nasdf]

LGTM. I think there are a few related issues that can be linked.

[fredcarle]

LGTM. Thanks for adding those tests.

[AndrewSisley]

praise: Thanks for these Islam, I think they are quite useful :)

[AndrewSisley]

LGTM, thanks Islam - you can probably link #2474 and #2475 to this PR and close them on merge.

[shahzadlone]

Thanks for adding these, you can resolve the related issues.

I do have a slight preference to avoid fixture / setup functions and have it all in the test as Andy suggested. But it's likely a subjective thing and i dont see that as a blocker.