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

Updating UpdateDependency test for all package managers + tests fixes #669

Merged
merged 5 commits into from
Mar 26, 2024

Conversation

eranturgeman
Copy link
Contributor

@eranturgeman eranturgeman commented Mar 21, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

This PR introduces improvements for UpdateDependency test:

  1. added missing testcase for pipenv
  2. added a check to verify dependency update in test cases where fix is suppose to happen
  3. added informative comments for test cases
  4. added a defer call for deleting tmp directories created during tests

…nformative comments, added a check to verify dependency ia actually updated as expected for every testcase in which fix is supported
@eranturgeman eranturgeman added ignore for release Automatically generated release notes improvement Automatically generated release notes labels Mar 21, 2024
@eranturgeman eranturgeman marked this pull request as draft March 21, 2024 08:10
…e-dependency-tests

# Conflicts:
#	packagehandlers/packagehandlers_test.go
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Mar 24, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 24, 2024
@eranturgeman eranturgeman marked this pull request as ready for review March 24, 2024 12:21
@eranturgeman eranturgeman changed the title Updating and fixing UpdateDependency test for all package managers Updating UpdateDependency test for all package managers + tests fixes Mar 24, 2024
@eranturgeman eranturgeman removed the improvement Automatically generated release notes label Mar 24, 2024
@eranturgeman eranturgeman added the safe to test Approve running integration tests on a pull request label Mar 24, 2024
@github-actions github-actions bot removed the safe to test Approve running integration tests on a pull request label Mar 24, 2024
Copy link
Contributor

👍 Frogbot scanned this pull request and found that it did not add vulnerable dependencies.


Copy link
Contributor

@EyalDelarea EyalDelarea left a comment

Choose a reason for hiding this comment

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

Looks great!
Just note that the previous tests also covered the whole "find descriptor file" process per technology, and if you'd like to do more of an integration test, this functionality should be checked as well.

@eranturgeman
Copy link
Contributor Author

Looks great! Just note that the previous tests also covered the whole "find descriptor file" process per technology, and if you'd like to do more of an integration test, this functionality should be checked as well.

Thanks @EyalDelarea !
I unified all the "find descriptor files" functions for the different PMs into a single function that is being checked separately. This change and new test were introduced in this PR: #656
Therefore I could take it our from the test that checks every PM

@eranturgeman eranturgeman removed the request for review from attiasas March 26, 2024 08:09
@eranturgeman eranturgeman merged commit 53aa387 into jfrog:dev Mar 26, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore for release Automatically generated release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants