Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vdk-core: fix buggy (false positive) connection unit test #841

Merged
merged 3 commits into from
May 27, 2022

Conversation

antoniivanov
Copy link
Collaborator

We have a unit test that checks the raw (or native) connection is being
saved properly - but the test was buggy since they are qual in a sense
they have the same attribute values (as there's only one "is_valid") .
But we care if it's the same instance. So we switch to "is"

Testing Done: After changing == to "is" ran the test and it failed (as
it should) and then fix the test.

Signed-off-by: Antoni Ivanov aivanov@vmware.com

We have a unit test that checks the raw (or native) connection is being
saved properly - but the test was buggy since they are qual in a sense
they have the same attribute values (as there's only one "is_valid") .
But we care if it's the same instance. So we switch to "is"

Testing Done: After changing == to "is" ran the test and it failed (as
it should) and then fix the test.

Signed-off-by: Antoni Ivanov <aivanov@vmware.com>
@antoniivanov antoniivanov force-pushed the person/aivanov/vdk-core-conn-test branch from b99d349 to 870e074 Compare May 21, 2022 12:52
@gageorgiev
Copy link
Contributor

The PR desc mentions fixing the test, but I only see replacement of == with is in the changes. Am I missing something?

@antoniivanov
Copy link
Collaborator Author

The PR desc mentions fixing the test, but I only see replacement of == with is in the changes. Am I missing something?

No, this is the fix in the test. It was false positive (it was passing even though it should not have) because native_conn.eq(managed_conn) (native_conn == managed_conn. Instead using "is" operator now would return true only if both vars refer to the same object

@antoniivanov antoniivanov enabled auto-merge (squash) May 25, 2022 21:05
@antoniivanov antoniivanov merged commit 94a79f4 into main May 27, 2022
@antoniivanov antoniivanov deleted the person/aivanov/vdk-core-conn-test branch May 27, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants