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

Use type safe asserts for (unit) testing #196

Closed
geryxyz opened this issue Jun 23, 2021 · 4 comments
Closed

Use type safe asserts for (unit) testing #196

geryxyz opened this issue Jun 23, 2021 · 4 comments

Comments

@geryxyz
Copy link
Collaborator

geryxyz commented Jun 23, 2021

Consider the following code chunk.

def test_extract_references_vuln_id():
    commit = Commit(
        commit_id="test_commit",
        repository="test_repository",
        cve_refs=["test_advisory_record", "another_advisory_record"],
    )
    advisory_record = AdvisoryRecord(vulnerability_id="test_advisory_record")
    result = extract_references_vuln_id(commit, advisory_record)
    assert result

My opinion is that this is a bad and discouraged practice, since the final assert disguise any implicit type conversions. For example in the case when the SUT is changed like in the case of #190

I suggest preferring the type safe version for every assert statement during testing (even in the case of boolean assertion). For example, like below.

def test_extract_references_vuln_id():
    commit = Commit(
        commit_id="test_commit",
        repository="test_repository",
        cve_refs=[
            "test_advisory_record",
            "another_advisory_record",
            "yet_another_advisory_record",
        ],
    )
    advisory_record = AdvisoryRecord(vulnerability_id="test_advisory_record")
    result = extract_references_vuln_id(commit, advisory_record)
    assert result is True
@copernico
Copy link
Contributor

Agreed!

@copernico
Copy link
Contributor

@geryxyz can we close this issue, based on your work in #194?
If not, what remains to be done?
Thanks!

@geryxyz
Copy link
Collaborator Author

geryxyz commented Jul 9, 2021

I think yes, at least I am unaware of any non-safe asserts in the code. For the record, they are quite tricky and time consuming to locate since in most of the case the test pass because the implicit conversions kick in. I.e. the problem is with the semantic of the test case, not the implementation.

@copernico
Copy link
Contributor

See #208

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants