[jira] [Commented] (CASSANDRA-14420) dtests not determining C* version correctly

["Blake Eggleston (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-14420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16459856#comment-16459856 ] 

Blake Eggleston commented on CASSANDRA-14420:
---------------------------------------------

I have a few notes:

* could we rename parse_dtest_config to dtest_config. While I realize the fixture function itself is doing the parsing, it seems a little strange to be passing the dtest config into the misc setup methods with that name
* you could probably set the parse_dtest_config fixture scope to something like module or session, so it's not reinstantiated for every test
* auth_test:TestAuthRoles.role has a mutable default argument which can lead to difficult to diagnose bugs. It's default should be None, then evaluated in the function body as {{options = options or {}}}
* I don't feel too strongly about this, but it looks like the parse_dtest_config argument is only used in a handful of fixture_dtest_setup_override implementations. Maybe it would be better to have a separate fixture setup for places where we need to consult the config? Otoh, passing the config into one of the main setup method seems like a reasonable thing to do. WDYT?
* There's a {{parse_dtest_config}} definition in user_functions_test:TestUserFunctions that's just behaving as a pass through. Is this left over from some debugging something, or is there a reason it's there? If it's doing something, could you add a comment explaining what?

> dtests not determining C* version correctly
> -------------------------------------------
>
>                 Key: CASSANDRA-14420
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14420
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Sam Tunnicliffe
>            Assignee: Sam Tunnicliffe
>            Priority: Major
>
> In the course of CASSANDRA-14134, the means of extracting the C* version under test before starting a cluster became broken. This is necessary in cases where we want to gate values in cassandra.yaml based on version, so a couple of tests are affected. The specifics are that the global {{CASSANDRA_VERSION_FROM_BUILD}} was hardcoded to '4.0' and the ways in which the various tests use it have meant that it was undetected until now.
> Also, the {{fixture_since}} which we use to implement the {{@since}} annotation is broken when a {{--cassandra-version}} is supplied, rather than {{--cassandra-dir}}, meaning testing against released versions from git isn't working right now.
> Tests directly affected:
>  * {{auth_test.py}} - CASSANDRA-13985 added some gating of yaml props and additional checks on CQL results based on the build version. These failed on 3.11, which is how this issue was uncovered, but they're also broken on 2.2 on builds.apache.org
>  * {{user_functions_test.py}} - gates setting a yaml property when version < 3.0. Failing on 2.2.
>  * {{upgrade_tests}} - a number of these use the variable, but I don't think they're actually being run at the moment.
>  * {{repair_tests/repair_test.py}}, {{replace_address_test.py}} & {{thrift_test}} all use the global, but only to verify that the version is not 3.9. As we're not running CI for that version, no-one noticed.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org