google / vulncode-db

Vulncode-DB project
https://www.vulncode-db.com
Apache License 2.0
570 stars 71 forks source link

Fix non-deterministic unit test failure #15

Closed evonide closed 4 years ago

evonide commented 4 years ago

In data/models/vulnerability.py we use joinedload which in some cases seems to lead to inconsistent unit tests SQL behavior where tests sometimes pass and sometimes fail. We need to investigate this further and address this as it's impacting all automatically run tests for push and pull requests to the repository.

evonide commented 4 years ago

Fixed in cceab1807d14713d0b761281352b2dd820e5985d. Main reason was this code: load_relationships1 = (joinedload(Vulnerability.commits).joinedload( VulnerabilityGitCommits.repository_files, VulnerabilityGitCommits.comments).joinedload(RepositoryFiles.comments))

Particularly: joinedload(VulnerabilityGitCommits.repository_files,VulnerabilityGitCommits.comments)

Sqlalchemy's joinedload has only one required parameter! The usage above will use the second parameter VulnerabilityGitCommits.comments for innerjoin behavior which leads to unpredictable behavior.

If it's evaluated to true Sqlalchemy will apply an "INNER JOIN" instead of "LEFT OUTER JOIN" on vulnerability commits and repository files leading to completely unexpected results.

Fix: Only provide one single parameter to joinedload and use multiple joinedload statements if required.