Skip to content

test: move language scanner tests to longtests #4322

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

Closed
terriko opened this issue Aug 8, 2024 · 1 comment · Fixed by #4327
Closed

test: move language scanner tests to longtests #4322

terriko opened this issue Aug 8, 2024 · 1 comment · Fixed by #4327
Labels
good first issue Good for newcomers

Comments

@terriko
Copy link
Contributor

terriko commented Aug 8, 2024

In #4321 I've noted that our slowest tests are all instances of test_language_package.

While we figure out the best way to improve performance for those, I do think it would be good right now to accept that those are long tests and move them accordingly so they only run if long tests are enabled. This should be doable by a beginner but it may take some reading to understand how pytest parameterization and skipping work.

Step by step instructions:

  • open up test/test_language_scanner.py and look at the function test_language_package
  • This is a parameterized set of tests, so you can't just use pytest.skipif directly. Take a look at how I skipped tests in https://github.com/intel/cve-bin-tool/pull/4319/files (it's at the bottom of that diff, the changes in test_scanner.py)
  • figure out how to change the parameters in test_language_package to achieve the same effect and do it!
  • don't forget to run black test/test_language_scanner.py to make sure you're style compliant. It'll also let you know if there's a syntax error
  • double-check your changes by trying to run pytest -k test_language_package on your local machine. If you get a bunch of sss notes showing skipped tests, you did it! (If you get a bunch of ... and the tests run, then try again)
@terriko terriko added the good first issue Good for newcomers label Aug 8, 2024
@terriko
Copy link
Contributor Author

terriko commented Aug 8, 2024

Since I've flagged this as a good first issue, here's the standard tips:

Short tips for new contributors:

  • cve-bin-tool's contributor docs
  • If you've contributed to open source but not this project, you might just want our checklist for a great pull request
  • cve-bin-tool uses https://www.conventionalcommits.org/ style for commit messages, and we have a test that checks the title of your pull request (PR). A good potential title for this one is in the title of this issue.
  • You can make an issue auto close by including a comment "fixes #ISSUENUMBER" in your PR comments where ISSUENUMBER is the actual number of the issue. This "links" the issue to the pull request.

Claiming issues:

  • You do not need to have an issue assigned to you before you work on it. To "claim" an issue either make a linked pull request or comment on the issue saying you'll be working on it.
  • If someone else has already commented or opened a pull request, assume it is claimed and find another issue to work on.
  • If it's been more than 1 week without progress, you can ask in a comment if the claimant is still working on it before claiming it yourself (give them at least 3 days to respond before assuming they have moved on).

muddi900 added a commit to muddi900/cve-bin-tool that referenced this issue Aug 9, 2024
terriko added a commit that referenced this issue Aug 15, 2024
* test: `test_language_package` is not skipped if LONG_TEST is disabled.[Issue #4322]

* Fixed the liniting error.

* changes to triage file

* Updated the mark to handle all input params.

---------

Co-authored-by: Terri Oda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant