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

feat(checker): New checker request - GNU emacs #2941

Merged
merged 5 commits into from
May 22, 2023
Merged

Conversation

bcieszko
Copy link
Contributor

@codecov-commenter
Copy link

codecov-commenter commented Apr 25, 2023

Codecov Report

Merging #2941 (03d9b46) into main (92d27dc) will decrease coverage by 0.66%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2941      +/-   ##
==========================================
- Coverage   82.85%   82.19%   -0.66%     
==========================================
  Files         676      678       +2     
  Lines       10665    10674       +9     
  Branches     1430     1430              
==========================================
- Hits         8836     8774      -62     
- Misses       1459     1533      +74     
+ Partials      370      367       -3     
Flag Coverage Δ
longtests 82.19% <100.00%> (+5.72%) ⬆️
win-longtests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/checkers/__init__.py 91.48% <ø> (ø)
test/test_checkers.py 95.45% <ø> (-4.55%) ⬇️
cve_bin_tool/checkers/emacs.py 100.00% <100.00%> (ø)
test/test_data/emacs.py 100.00% <100.00%> (ø)

... and 22 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Looking great! Can you add at least one test file and test to go with the checker?

Here's an example of a different checker if you're not sure what that should look like:

https://github.com/intel/cve-bin-tool/pull/2862/files

Basically you'll add some data into test/test_data and a file into test/condensed_downloads . There's some instructions here on how to handle the "real file" test: https://github.com/intel/cve-bin-tool/blob/main/test/README.md#adding-new-tests-signature-tests-against-real-files

(The "condensed downloads" are basically compressed files of strings extracted from the binary so that we're not distributing full vulnerable binaries of open source packages but can still run regression tests.)

Let us know if you get stuck anywhere!

@bcieszko
Copy link
Contributor Author

Sure, will add a test.

@bcieszko
Copy link
Contributor Author

Tests added :)

@bcieszko bcieszko requested a review from terriko April 27, 2023 12:33
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

The tests are detecting libtiff and pango in the ubuntu binary (and only the ubuntu binary). I've put instructions for how to fix it if that's expected and if it's unexpected inline. (short term it's the same solution: add libtiff and pango to other_products, longer term it's possible those signatures are a bit overzealous in matching and we could file a separate issue for that)

test/test_data/emacs.py Show resolved Hide resolved
@terriko terriko linked an issue Apr 27, 2023 that may be closed by this pull request
* feat(checker): New checker request - GNU emacs
* closes intel#2921

Signed-off-by: Bartlomiej Cieszkowski <bartlomiej.cieszkowski@intel.com>
Signed-off-by: Przemyslaw Romaniak <przemyslaw.romaniak@intel.com>
workaround for false detection

Co-authored-by: Terri Oda <terri@toybox.ca>
Signed-off-by: Bartlomiej Cieszkowski <bartlomiej.cieszkowski@intel.com>
@bcieszko
Copy link
Contributor Author

bcieszko commented May 1, 2023

cleaned up black checker

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

This looks ready to me; I'm going to update the branch to get the linters to run since re-running them yesterday doesn't seem to have worked.

test/test_data/emacs.py Show resolved Hide resolved
test/test_data/emacs.py Outdated Show resolved Hide resolved
@bcieszko
Copy link
Contributor Author

@terriko should i update branch? to make checkers run?

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

There we go. I think you got hit during a period after we turned off the self-hosted runners where some tests just failed. Working now, so time to merge!

@terriko terriko merged commit f008d77 into intel:main May 22, 2023
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.

feat(checker): New checker request - GNU emacs
3 participants