Fix failing tests due to query_qp attribute mask

[ygoldamzn]

Move attribute query out of base create_players.
Not all vendors support all query_qp attribute mask values and this breaks all tests. It is only required for a single test suite, so move it there as it was prior to PR #1349

Fixes: 78f6a65 ("tests: Remove code duplication")

[mrgolin]

@rleon Can you please review / merge this? All tests are currently failing for EFA.

[EdwardSro]

What do you get when querying with this mask? EOPNOTSUPP?
How does moving this back to test_mr help? it would still fail the whole test_mr suit (except if you skip them internally).
Maybe a better idea is to wrap it in base with try-except and leave them empty (None) in case of EOPNOTSUPP or relevant errno?

[ygoldamzn]

What do you get when querying with this mask? EOPNOTSUPP? How does moving this back to test_mr help? it would still fail the whole test_mr suit (except if you skip them internally). Maybe a better idea is to wrap it in base with try-except and leave them empty (None) in case of EOPNOTSUPP or relevant errno?

Yes, we get an EOPNOTSUPP.

Moving it to test_mr helps because that's the only suite that ever uses the result of the query, or even bothers to initialize the members the result is assigned to. This makes skipping tests that rely on not supported operations semantically more accurate.
Wrapping it with try-except at the base would make test_mr fail someplace which is further away from where the error occurred, which makes it harder to debug. It also keeps something in the base class which is only relevant for one derived class.

[EdwardSro]

Moving it to test_mr helps because that's the only suite that ever uses the result of the query, or even bothers to initialize the members the result is assigned to. This makes skipping tests that rely on not supported operations semantically more accurate. Wrapping it with try-except at the base would make test_mr fail someplace which is further away from where the error occurred, which makes it harder to debug. It also keeps something in the base class which is only relevant for one derived class.

If so, I'll go with a more specific solution and add the query only to the required tests (which are just two: test_mr_rereg_pd and test_mr_rereg_addr) to minimize the tests you have to skip.

[ygoldamzn]

Moving it to test_mr helps because that's the only suite that ever uses the result of the query, or even bothers to initialize the members the result is assigned to. This makes skipping tests that rely on not supported operations semantically more accurate. Wrapping it with try-except at the base would make test_mr fail someplace which is further away from where the error occurred, which makes it harder to debug. It also keeps something in the base class which is only relevant for one derived class.

If so, I'll go with a more specific solution and add the query only to the required tests (which are just two: test_mr_rereg_pd and test_mr_rereg_addr) to minimize the tests you have to skip.

Sounds good, updated code to adopt this proposal.

[EdwardSro]

Thanks