cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andres de la Peña (Jira) <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-16841) Unexpectedly ignored dtests
Date Fri, 03 Sep 2021 19:57:00 GMT

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

Andres de la Peña edited comment on CASSANDRA-16841 at 9/3/21, 7:56 PM:
------------------------------------------------------------------------

Thanks for the patch. Finding those skipped tests is a very good catch. Overall the patch
looks good to me, I have left some minor suggestions on the PR.
{quote}Treat --only-resource-intensive-tests in the same way as --force-resource-intensive-tests,
so it will be enough to just specify it even with no sufficient resources.
{quote}
I'm not sure about this, IMO it makes sense the way it currently works. The flag {{\-\-only-resource-intensive-tests}}
selects the tests to run and {{\-\-force-resource-intensive-tests}} disables the safety mechanism
that prevents running the tests if there aren't enough resources available.

On one hand, I understand that current meaning of {{\-\-only-resource-intensive-tests}} without
{{\-\-force-resource-intensive-tests}} is "run the resource intensive tests but only if you
have resources to do so". I guess that a (convoluted) example use case for this could be having
a CircleCI job for running resource intensive tests. This job would success in LOWRES and
MIDRES without running any tests due to the lack of resources, and it would actually run the
tests in HIGHRES.

On the other hand, I understand that in most cases when you use {{\-\-only-resource-intensive-tests}}
probably you are going to want to run with or without the resources. I don't think that any
of the approaches is wrong or much better than the other, so I'm more inclined to preserve
the current behaviour. We can always add some more information in the description of the flags
if we think that this is going to be confusing for users. What do you think? Am I missing
something?


was (Author: adelapena):
Thanks for the patch. Finding those skipped tests is a very good catch. Overall the patch
looks good to me, I have left some minor suggestions on the PR.
{quote}Treat --only-resource-intensive-tests in the same way as --force-resource-intensive-tests,
so it will be enough to just specify it even with no sufficient resources.
{quote}
I'm not sure about this, IMO it makes sense the way it currently works. The flag {{--only-resource-intensive-tests}}
selects the tests to run and {{--force-resource-intensive-tests}} disables the safety mechanism
that prevents running the tests if there aren't enough resources available.

On one hand, I understand that current meaning of {{--only-resource-intensive-tests}} without
{{--force-resource-intensive-tests}} is "run the resource intensive tests but only if you
have resources to do so". I guess that a (convoluted) example use case for this could be having
a CircleCI job for running resource intensive tests. This job would success in LOWRES and
MIDRES without running any tests due to the lack of resources, and it would actually run the
tests in HIGHRES.

On the other hand, I understand that in most cases when you use {{--only-resource-intensive-tests}}
probably you are going to want to run with or without the resources. I don't think that any
of the approaches is wrong or much better than the other, so I'm more inclined to preserve
the current behaviour. We can always add some more information in the description of the flags
if we think that this is going to be confusing for users. What do you think? Am I missing
something?

> Unexpectedly ignored dtests
> ---------------------------
>
>                 Key: CASSANDRA-16841
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-16841
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Test/dtest/python
>            Reporter: Ruslan Fomkin
>            Assignee: Ruslan Fomkin
>            Priority: Normal
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> An issue, which I was hit:
> When one class in a dtest file is marked as resource intensive, then all tests in all
classes are treated as resource intensive. For example, [repair_tests/repair_test.py|https://github.com/apache/cassandra-dtest/blob/trunk/repair_tests/repair_test.py] contains
three classes and the last class is marked as resource intensive:
> {code:java}
> @pytest.mark.resource_intensive
> class TestRepairDataSystemTable(Tester):
> {code}
> So if I try to run an unmarked class: 
> {code:java}
> pytest --cassandra-dir=../cassandra repair_tests/repair_test.py::TestRepair --collect-only
--skip-resource-intensive-tests
> {code}
> then all tests are ignored
> {code:java}
> collected 36 items / 36 deselected 
> {code}
> This is because a test is treated to be marked if any class in the same file has the
mark. This bug was introduced in the fix of CASS-16399. Before only upgrade tests had such
behaviour, i.e., if a class is marked as upgrade test, then all tests are upgrade test in
the file.
>  
> This bug, for example, means that if the same file contains one class marked with vnodes
and another class with no_vnodes, then no tests will be executed in the file.
> I also noticed another issue that If a test run is executed with the argument {{-only-resource-intensive-tests}}
and there is no sufficient resources for resource intensive tests, then no tests were executed. Thus
it was necessary to provide {{-force-resource-intensive-tests}} in addition.
> Suggestions for the solutions:
>  # Require to mark each class and remove the special case of upgrade tests. This will
simplify the implementation and might be more obvious for new comers.
>  # Treat {{-only-resource-intensive-tests}} in the same way as {{-force-resource-intensive-tests}},
so it will be enough to just specify it even with no sufficient resources.
>  
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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


Mime
View raw message