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

Fixes in Debian Index Visitor function. #41

Merged
merged 6 commits into from
Mar 8, 2023

Conversation

35C4n0r
Copy link
Contributor

@35C4n0r 35C4n0r commented Feb 21, 2023

Closes #23
Closes #34

  1. Gzip file content will read as text instead of bytes. String version Will be passed instead of Version Object.
  2. .dsc and other files are now handled correctly.
  3. New testfilles are added.

Signed-off-by: 35C4n0r jaykumar20march@gmail.com

Signed-off-by: 35C4n0r <jaykumar20march@gmail.com>
@35C4n0r
Copy link
Contributor Author

35C4n0r commented Feb 21, 2023

@JonoYang please review this.
These are the tests for this

  • test_debian.py::DebianLSLRTest::test_DebianDirectoryIndexVisitor_from_debian
  • test_debian.py::DebianLSLRTest::test_DebianDirectoryIndexVisitor_from_ubuntu

Copy link
Member

@JonoYang JonoYang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, I left a few comments.

minecode/visitors/debian.py Show resolved Hide resolved
minecode/tests/test_debian.py Outdated Show resolved Hide resolved
Test files are now kept as normal readable files and are converted to a .gz file at a temporary location on running the tests.
Signed-off-by: 35C4n0r <jaykumar20march@gmail.com>
minecode/tests/test_debian.py Outdated Show resolved Hide resolved
… redundancy.

Signed-off-by: 35C4n0r <jaykumar20march@gmail.com>
minecode/tests/test_debian.py Outdated Show resolved Hide resolved
@@ -0,0 +1,87 @@
.:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you replace the ls-lR_debian and ls-lR_ubuntu files with the original ones from https://github.com/nexB/purldb/tree/main/minecode/tests/testfiles/debian/lslr, extracted from the .gz files? We want to use the same test data as before to see if we get the same expected results to ensure the fix that you created does not introduce any changes to how the function worked before. If you want, you can create an additional test with your new test ls-lR data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonoYang I changed the files to the original ones but, i had to make changes to the expected.json files as there were a few errors.
Example:
all the purls in https://github.com/nexB/purldb/blob/main/minecode/tests/testfiles/debian/lslr/ls-lR_debian.gz-expected.json, are in wrong format ex: pkg:deb/debian/asterisk-mobile@1.6.2.1-1?arch=amd64.deb.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@35C4n0r The idea is to use the previous ls-lR files and then update the expected JSON results.

Signed-off-by: 35C4n0r <jaykumar20march@gmail.com>
Signed-off-by: 35C4n0r <jaykumar20march@gmail.com>
def test_DebianDirectoryIndexVisitor_from_debian(self):
uri = 'http://ftp.debian.org/debian/ls-lR.gz'
test_loc = self.get_test_loc('debian/lslr/ls-lR_debian.gz')
temp_gz_location = self.get_tmp_gz_file('debian/lslr/ls-lR_debian')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test still fails: https://github.com/nexB/purldb/actions/runs/4310196055/jobs/7518392136#step:6:764

You need to do something like this:

Suggested change
temp_gz_location = self.get_tmp_gz_file('debian/lslr/ls-lR_debian')
test_loc = self.get_test_loc('debian/lslr/ls-lR_debian')
temp_gz_location = self.get_tmp_gz_file(test_loc)
with patch('requests.get') as mock_http_get:
mock_http_get.return_value = mocked_requests_get(uri, temp_gz_location)
uris, _, _ = debian_visitor.DebianDirectoryIndexVisitor(uri)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @JonoYang, I've updated the tests, and the original lsLr files are used, and the json result have been updated accordingly.

Signed-off-by: 35C4n0r <jaykumar20march@gmail.com>
@JonoYang JonoYang merged commit 3d159bf into aboutcode-org:main Mar 8, 2023
@JonoYang
Copy link
Member

JonoYang commented Mar 8, 2023

@35C4n0r Everything looks great! Thank you for your contribution!

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