Remove self assert #507

Lorak-mmk · 2025-07-16T16:33:50Z

One of the steps to improving integration tests is migrating to pytest (== not having classes derived from unittest.TestCase). Why? Such classes don't allow us to use pytest niceties like fixtures.
A prerequisite to changing our classes is removing usage of things like self.assertEquals. The recommended way for asserts in pytest is just using plain asserts - pytest does some bytecode magic under the hood to produce nice error messages.
This is what this PR does - it remove all (I hope) usages of unittest asserts.
There were some that I could not translate easily into plain asserts - for that I created utility functions that call unittest assertions under the hood. It will make it easier to translate in the future.

Pre-review checklist

I have split my patch into logically separate commits.
All commit messages clearly explain what they change and why.
~~I added relevant tests for new features and bug fixes.~~
All commits compile, pass static checks and pass test.
PR description sums up the changes and reasons why they should be introduced.
~~I have provided docstrings for the public items that I want to introduce.~~
~~I have adjusted the documentation in ./docs/source/~~.
I added appropriate Fixes: annotations to PR description.

No need for it - cluster is already stored in __init__.py

Lorak-mmk · 2025-07-17T18:47:10Z

Addressed Copilot comments.

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertEqual($A, $B)' --rewrite 'assert $A == $B' --lang python ./tests -U ast-grep run --pattern 'self.assertEqual($A, $B, $C)' --rewrite 'assert $A == $B, $C' --lang python ./tests -U ``` Some manual fixes were required for edge cases.

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertIsNone($A)' --rewrite 'assert $A is None' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertIn($A, $B)' --rewrite 'assert $A in $B' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertIsInstance($A, $B)' --rewrite 'assert isinstance($A, $B)' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertNotEqual($A, $B)' --rewrite 'assert $A != $B' --lang python ./tests -U ast-grep run --pattern 'self.assertNotEqual($A, $B, $C)' --rewrite 'assert $A != $B, $C' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertTrue($A)' --rewrite 'assert $A' --lang python ./tests -U ast-grep run --pattern 'self.assertTrue($A, $B)' --rewrite 'assert $A, $B' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertNotIn($A, $B)' --rewrite 'assert $A not in $B' --lang python ./tests -U ast-grep run --pattern 'self.assertNotIn($A, $B, $C)' --rewrite 'assert $A not in $B, $C' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertFalse($A)' --rewrite 'assert not $A' --lang python ./tests -U ast-grep run --pattern 'self.assertFalse($A, $B)' --rewrite 'assert not $A, $B' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertLessEqual($A, $B)' --rewrite 'assert $A <= $B' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertGreater($A, $B)' --rewrite 'assert $A > $B' --lang python ./tests -U ast-grep run --pattern 'self.assertGreater($A, $B, $C)' --rewrite 'assert $A > $B, $C' --lang python ./tests -U ```

>Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertGreaterEqual($A, $B)' --rewrite 'assert $A >= $B' --lang python ./tests -U ast-grep run --pattern 'self.assertGreaterEqual($A, $B, $C)' --rewrite 'assert $A >= $B, $C' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertIsNotNone($A)' --rewrite 'assert $A is not None' --lang python ./tests -U ast-grep run --pattern 'self.assertIsNotNone($A, $B)' --rewrite 'assert $A is not None, $B' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertHasAttr($A, $B)' --rewrite 'assert hasattr($A, $B)' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertIs($A, $B)' --rewrite 'assert $A is $B' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertLess($A, $B)' --rewrite 'assert $A < $B' --lang python ./tests -U ast-grep run --pattern 'self.assertLess($A, $B, $C)' --rewrite 'assert $A < $B, $C' --lang python ./tests -U ```

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertIsNot($A, $B)' --rewrite 'assert $A is not $B' --lang python ./tests -U ```

This is part of migration effort to pytest. This commit was done by hand. I introduced a new util function, and replaced the usages of self.assertRegex with it. Why this way? If we ever need better error messages for those asserts, we'll be able to do this by just modifying this function, not all callsites.

Replaces usages of self.assertListEqual and other collection-related asserts with newly introduced util methods. Those new metods still call unittest ones under the hood, so why do this? As with previous commits, the goal is to migrate to pytest, which requires no using subclasses of TestCase. Offocial migration tool (unittest2pytest) replaces those methods with just equality asserts. This does not preseve semantics perfectly, so I opted for a safer option for now. We can of course change that in the future.

pytest.approx is more capable - it can replicate all functionality of assertAlmostEqual, but also can perform comparisons with relative error.

This method took a assertEqual callback, but it was always self.assertEqual. This commit removes this unnecessary indirection by replacing the callback with plain assert.

I decided to not replace it with simple assert x.startswith(y) because the diff that the function produces is superior to default error message.

They can be replaced, depending on situation, with: - assert - pytest.raises - pytest.fail

Replaces all uses of self.assertRaises and self.assertRaisesRegex with pytest.raises. I started this commit with command `unittest2pytest -f self_assert ./tests -w -n`. After it finished, I manually reviewed all differences, and performed fixes where necessary / useful. There were two types of fixes: - When using output value of context manager (with ... as X), unittest exposes the thrown exception as X.exception, while pytest as X.value. - In few places there were calls like `self.assertRaises(T, func, **{'arg': val, ...}`. Passing args as dict with ** is not pretty, so I desugared this into directly passing named args.

Copilot

Pull Request Overview

This PR modernizes test assertions by replacing unittest-style assertions with plain pytest assertions throughout the codebase. This is a prerequisite step for migrating integration tests away from unittest.TestCase inheritance to enable the use of pytest fixtures and other pytest features.

Key changes:

Replaces self.assertEqual(), self.assertNotEqual(), self.assertTrue(), etc. with plain assert statements
Converts self.assertRaises() to pytest.raises() context managers
Updates some custom assertion helpers to be standalone functions

Reviewed Changes

Copilot reviewed 122 out of 135 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
tests/unit/test_query.py	Converts batch statement test assertions to plain asserts
tests/unit/test_protocol_features.py	Updates protocol feature parsing test assertions
tests/unit/test_protocol.py	Converts protocol message and continuous paging test assertions
tests/unit/test_policies.py	Extensive conversion of policy-related test assertions
tests/unit/test_parameter_binding.py	Updates parameter binding test assertions
tests/unit/test_orderedmap.py	Converts OrderedMap test assertions
tests/unit/test_metadata.py	Updates metadata parsing and validation test assertions
tests/unit/test_marshalling.py	Converts serialization/deserialization test assertions
tests/unit/test_host_connection_pool.py	Updates connection pool test assertions
tests/unit/test_exception.py	Converts exception handling test assertions
tests/unit/test_endpoints.py	Updates endpoint-related test assertions
tests/unit/test_control_connection.py	Converts control connection test assertions
tests/unit/test_connection.py	Updates connection handling test assertions
tests/unit/test_concurrent.py	Converts concurrent execution test assertions
tests/unit/test_cluster.py	Updates cluster configuration test assertions
tests/unit/test_auth.py	Converts authentication test assertions
tests/unit/io/	Updates I/O reactor test assertions
tests/unit/cython/	Converts Cython extension test assertions
tests/unit/cqlengine/	Updates CQLEngine test assertions
tests/unit/column_encryption/	Converts encryption policy test assertions
tests/unit/advanced/	Updates advanced feature test assertions

Copilot · 2025-07-17T18:48:54Z

tests/unit/test_policies.py

@@ -869,8 +878,8 @@ def test_basic_responses(self):
        """

        conviction_policy = SimpleConvictionPolicy(1)
-        self.assertEqual(conviction_policy.add_failure(1), True)
-        self.assertEqual(conviction_policy.reset(), None)
+        assert conviction_policy.add_failure(1) == True


Comparing boolean values with '== True' is redundant. Use 'assert conviction_policy.add_failure(1)' instead.

Suggested change

assert conviction_policy.add_failure(1) == True

assert conviction_policy.add_failure(1)

Copilot · 2025-07-17T18:48:54Z

tests/unit/test_policies.py

-        self.assertEqual(conviction_policy.add_failure(1), True)
-        self.assertEqual(conviction_policy.reset(), None)
+        assert conviction_policy.add_failure(1) == True
+        assert conviction_policy.reset() == None


Comparing with 'None' should use 'is None' instead of '== None' for clarity and consistency.

Suggested change

assert conviction_policy.reset() == None

assert conviction_policy.reset() is None

Copilot · 2025-07-17T18:48:54Z

tests/unit/test_policies.py

        for i, delay in enumerate(schedule):
-            self.assertEqual(delay, delay)
+            assert delay == delay


This assertion compares a variable with itself and will always pass. This appears to be a copy-paste error where the loop variable should be compared with the expected delay value.

This is a great comment. Fixed.

Copilot · 2025-07-17T18:48:55Z

tests/unit/test_policies.py

+        assert retry == RetryPolicy.RETHROW
+        assert consistency == None

        # if we didn't get enough responses, rethrow
        retry, consistency = policy.on_read_timeout(
            query=None, consistency=ONE, required_responses=2, received_responses=1,
            data_retrieved=True, retry_num=0)
-        self.assertEqual(retry, RetryPolicy.RETHROW)
-        self.assertEqual(consistency, None)
+        assert retry == RetryPolicy.RETHROW
+        assert consistency == None

        # if we got enough responses, but also got a data response, rethrow
        retry, consistency = policy.on_read_timeout(
            query=None, consistency=ONE, required_responses=2, received_responses=2,
            data_retrieved=True, retry_num=0)
-        self.assertEqual(retry, RetryPolicy.RETHROW)
-        self.assertEqual(consistency, None)
+        assert retry == RetryPolicy.RETHROW
+        assert consistency == None

        # we got enough responses but no data response, so retry
        retry, consistency = policy.on_read_timeout(
            query=None, consistency=ONE, required_responses=2, received_responses=2,
            data_retrieved=False, retry_num=0)
-        self.assertEqual(retry, RetryPolicy.RETRY)
-        self.assertEqual(consistency, ONE)
+        assert retry == RetryPolicy.RETRY


[nitpick] Consider using 'is' instead of '==' when comparing with enum constants for better performance and clarity.

Copilot · 2025-07-17T18:48:55Z

tests/unit/test_policies.py

+        assert consistency == None

        # if we didn't get enough responses, rethrow
        retry, consistency = policy.on_read_timeout(
            query=None, consistency=ONE, required_responses=2, received_responses=1,
            data_retrieved=True, retry_num=0)
-        self.assertEqual(retry, RetryPolicy.RETHROW)
-        self.assertEqual(consistency, None)
+        assert retry == RetryPolicy.RETHROW
+        assert consistency == None

        # if we got enough responses, but also got a data response, rethrow
        retry, consistency = policy.on_read_timeout(
            query=None, consistency=ONE, required_responses=2, received_responses=2,
            data_retrieved=True, retry_num=0)
-        self.assertEqual(retry, RetryPolicy.RETHROW)
-        self.assertEqual(consistency, None)
+        assert retry == RetryPolicy.RETHROW
+        assert consistency == None


Use 'is None' instead of '== None' for comparing with None.

Copilot · 2025-07-17T18:48:55Z

tests/unit/test_metadata.py

+        assert is_valid_name(None) == False
+        assert is_valid_name('test') == True
+        assert is_valid_name('Test') == False
+        assert is_valid_name('t_____1') == True
+        assert is_valid_name('test1') == True
+        assert is_valid_name('1test1') == False

        invalid_keywords = cassandra.metadata.cql_keywords - cassandra.metadata.cql_keywords_unreserved
        for keyword in invalid_keywords:
-            self.assertEqual(is_valid_name(keyword), False)
+            assert is_valid_name(keyword) == False


Comparing boolean values with '== False' is redundant. Use 'assert not is_valid_name(None)' instead.

Copilot · 2025-07-17T18:48:55Z

tests/unit/test_parameter_binding.py

+        assert self.bound.values[-1] == None

        old_values = self.bound.values
        self.bound.bind((0, 0, 0, None))
-        self.assertIsNot(self.bound.values, old_values)
-        self.assertEqual(self.bound.values[-1], None)
+        assert self.bound.values is not old_values
+        assert self.bound.values[-1] == None


Use 'is None' instead of '== None' for comparing with None.

Lorak-mmk · 2025-07-17T18:54:51Z

I'm inclined to ignore the comments regarding == vs is and similar things. This is not what the PR is about, and it doesn't really introduce those issues - it just exposes them more explicitly.
If we introduce some linter (like Ruff) in the future, it should take care of that.

This test compared the variable with itself. This is obviously an error. I think it meant to compare it with the configured delay.

test_tablets: Remove CCM_CLUSTER global

2e8d427

No need for it - cluster is already stored in __init__.py

Lorak-mmk self-assigned this Jul 17, 2025

Lorak-mmk force-pushed the remove-self-assert branch from ca36fb1 to 76e3506 Compare July 17, 2025 15:09

Lorak-mmk requested review from dkropachev and Copilot and removed request for dkropachev July 17, 2025 17:57

This comment was marked as outdated.

Sign in to view

Lorak-mmk force-pushed the remove-self-assert branch from 76e3506 to 528c589 Compare July 17, 2025 18:47

Lorak-mmk added 20 commits July 17, 2025 20:47

Replace self.assertIsNone with plain assert

e966840

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertIsNone($A)' --rewrite 'assert $A is None' --lang python ./tests -U ```

Replace self.assertIn with plain assert

a34dfe2

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertIn($A, $B)' --rewrite 'assert $A in $B' --lang python ./tests -U ```

Replace self.assertIsInstance with plain assert

3198417

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertIsInstance($A, $B)' --rewrite 'assert isinstance($A, $B)' --lang python ./tests -U ```

Replace self.assertLessEqual with plain assert

aae2fcc

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used command: ``` ast-grep run --pattern 'self.assertLessEqual($A, $B)' --rewrite 'assert $A <= $B' --lang python ./tests -U ```

Replace self.assertHasAttr with plain assert

b86bf39

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertHasAttr($A, $B)' --rewrite 'assert hasattr($A, $B)' --lang python ./tests -U ```

Replace self.assertIs with plain assert

493d15d

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertIs($A, $B)' --rewrite 'assert $A is $B' --lang python ./tests -U ```

Replace self.assertIsNot with plain assert

bde333b

Part of effort to migrate to pytest. Change was done automatically, using ast-grep tool. Used commands: ``` ast-grep run --pattern 'self.assertIsNot($A, $B)' --rewrite 'assert $A is not $B' --lang python ./tests -U ```

Replace self.assertAlmostEqual with pytest.approx

cc18e01

pytest.approx is more capable - it can replicate all functionality of assertAlmostEqual, but also can perform comparisons with relative error.

verify_iterator_data: Use plain assert

7a3e725

This method took a assertEqual callback, but it was always self.assertEqual. This commit removes this unnecessary indirection by replacing the callback with plain assert.

Lorak-mmk added 17 commits July 17, 2025 20:47

test_types: Use new assertEqual for callbacks

8327102

assert_startswith_diff: Move to utils

6e61283

I decided to not replace it with simple assert x.startswith(y) because the diff that the function produces is superior to default error message.

Replace commented out unittest asserts with plain asserts

fc6082b

bytesio_testhelper: Use plain asserts for equality testing

ecfc813

bytesio_testhelper: Remove assert_raises argument

c97f3db

cython/test_datetime_from_timestamp: Use plain assert

48da238

cython/test_types: Use plain asserts

6ce4940

test_response_future: Use util asserts

bb9e88f

Replace assert_startswith with plain assert

381e861

Remove unused assertion functions

a9cfbc1

assert_query_count_equals: Use pytest.fail

2fbcb9a

test_types: Remove unittests asserts

8bf9742

tests: Get rid of self.fail calls

adb7144

They can be replaced, depending on situation, with: - assert - pytest.raises - pytest.fail

check_lookup: use plain assert

23b7bf8

integration/standard/test_metadata: Plain asserts in FunctionTest

295e365

execute_count: Use plain assert

a001669

Lorak-mmk requested a review from Copilot July 17, 2025 18:47

Copilot AI reviewed Jul 17, 2025

View reviewed changes

Lorak-mmk force-pushed the remove-self-assert branch from 528c589 to cbe760f Compare July 17, 2025 18:58

Lorak-mmk added 2 commits July 17, 2025 20:58

assert_quiescent_pool_state: Use plain asserts

238e33c

Test ConstantReconnectionPolicy: Fix the copy-paste bug

cbe760f

This test compared the variable with itself. This is obviously an error. I think it meant to compare it with the configured delay.

dkropachev approved these changes Jul 18, 2025

View reviewed changes

dkropachev merged commit c233c26 into scylladb:master Jul 18, 2025
25 of 26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove self assert #507

Remove self assert #507

Uh oh!

Lorak-mmk commented Jul 16, 2025 •

edited

Loading

Uh oh!

This comment was marked as outdated.

Uh oh!

Lorak-mmk commented Jul 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Lorak-mmk Jul 17, 2025

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Copilot AI Jul 17, 2025

Uh oh!

Lorak-mmk commented Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

	assert conviction_policy.add_failure(1) == True
	assert conviction_policy.add_failure(1)

	assert conviction_policy.reset() == None
	assert conviction_policy.reset() is None

Remove self assert #507

Remove self assert #507

Uh oh!

Conversation

Lorak-mmk commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-review checklist

Uh oh!

This comment was marked as outdated.

Uh oh!

Lorak-mmk commented Jul 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Lorak-mmk Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

Lorak-mmk commented Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

Lorak-mmk commented Jul 16, 2025 •

edited

Loading