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

Remove compiler dependency: added new mapping test data format #716

Merged
merged 36 commits into from Jun 10, 2020

Conversation

Niraj-Kamdar
Copy link
Contributor

@Niraj-Kamdar Niraj-Kamdar commented May 29, 2020

I have created separate python file to store mapping test data. I have done this because it would be nice to keep data separated from the code.
We can import whole array from the mapping_test_data file and use it directly in test_scanner.
@terriko, @pdxjohnny needs your opinions on this new structure.

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2020

Codecov Report

Merging #716 into master will increase coverage by 0.94%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #716      +/-   ##
==========================================
+ Coverage   87.71%   88.65%   +0.94%     
==========================================
  Files          76      125      +49     
  Lines        2173     2266      +93     
  Branches      264      263       -1     
==========================================
+ Hits         1906     2009     +103     
+ Misses        208      201       -7     
+ Partials       59       56       -3     
Flag Coverage Δ
#longtests 88.65% <100.00%> (+0.94%) ⬆️
Impacted Files Coverage Δ
cve_bin_tool/checkers/__init__.py 100.00% <ø> (ø)
cve_bin_tool/checkers/ffmpeg.py 100.00% <ø> (ø)
test/test_checkers.py 100.00% <ø> (ø)
test/test_scanner.py 95.94% <ø> (+4.51%) ⬆️
cve_bin_tool/checkers/gnutls.py 100.00% <100.00%> (ø)
cve_bin_tool/checkers/libcurl.py 100.00% <100.00%> (ø)
test/test_cli.py 100.00% <100.00%> (ø)
test/test_data/__init__.py 100.00% <100.00%> (ø)
test/test_data/bash.py 100.00% <100.00%> (ø)
test/test_data/binutils.py 100.00% <100.00%> (ø)
... and 103 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f39bd3...2e4cada. Read the comment docs.

@pdxjohnny
Copy link
Member

This format looks okay to me

@Niraj-Kamdar

This comment has been minimized.

@Niraj-Kamdar Niraj-Kamdar force-pushed the master branch 2 times, most recently from 0f00750 to 7e81dd7 Compare June 2, 2020 16:30
Update README.md
Copy link
Member

@pdxjohnny pdxjohnny left a comment

Choose a reason for hiding this comment

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

  • Do you mean to still have test/assets/test-curl-7.34.0.out and test/assets/test-curl-7.34.0.out?
  • Where are the tests being run?
  • Do are the common strings that we usually find when compiling with gcc being added to the fake binaries?

test/README.md Show resolved Hide resolved
test/test_file.py Show resolved Hide resolved
@pdxjohnny
Copy link
Member

Looks like you're making great progress here!

@Niraj-Kamdar
Copy link
Contributor Author

Niraj-Kamdar commented Jun 3, 2020

@pdxjohnny I am keeping two compiled binaries in assets because, testing our python strings module against fake binary won't be really useful. We have to make sure that our strings implementation give same result as strings command on real binaries.

No, I am not going to add those common strings in every binary file we generate. I think it would be nice to have a separate test for that so that developer get more details.

@Niraj-Kamdar
Copy link
Contributor Author

This will close issue #638, #669 and #668.

@Niraj-Kamdar
Copy link
Contributor Author

Niraj-Kamdar commented Jun 4, 2020

We don't have following test data.
no package for: bluetoothctl
no package for: gimp
no package for: kerberos
no mapping for: kerberos_5
no package or maping for: openssh-server
We should create issue for these and label it as first good issue.

Copy link
Member

@pdxjohnny pdxjohnny left a comment

Choose a reason for hiding this comment

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

Can you make those issues you were talking about? Then this looks good, also, see my comment above ^

@Niraj-Kamdar
Copy link
Contributor Author

Niraj-Kamdar commented Jun 4, 2020

  • Do you mean to still have test/assets/test-curl-7.34.0.out and test/assets/test-curl-7.34.0.out?
  • Where are the tests being run?
  • Do are the common strings that we usually find when compiling with gcc being added to the fake binaries?

Here is the answer for these comments: #716 (comment).
And I am making those issues :)

@SaurabhK122
Copy link
Contributor

@Niraj-Kamdar, we have package test data for gimp, please see the commits in PR #726.

@Niraj-Kamdar
Copy link
Contributor Author

@Niraj-Kamdar, we have package test data for gimp, please see the commits in PR #726.

I might have missed it. thanks!

@pdxjohnny pdxjohnny merged commit 3a80036 into intel:master Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants