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

Upgrade to grype 0.16.0 and add tests #102 #112

Merged
merged 10 commits into from Aug 19, 2021

Conversation

kzantow
Copy link
Contributor

@kzantow kzantow commented Aug 19, 2021

This fixes: #102

Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
Signed-off-by: Keith Zantow <kzantow@gmail.com>
@kzantow kzantow changed the title Upgrade to grype 0.16.0 and add tests Upgrade to grype 0.16.0 and add tests #102 Aug 19, 2021
@kzantow kzantow requested a review from a team August 19, 2021 19:28
Copy link
Contributor

@luhring luhring 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 amazing! Just a few questions for you

- uses: actions/checkout@v2
- run: echo $(uname -a)
- name: Check for npm (so make test works)
run: |
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it's worth trying to have all of these steps be available as make tasks? (For local use)

Copy link
Contributor Author

@kzantow kzantow Aug 19, 2021

Choose a reason for hiding this comment

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

these are not required for local use, running make test will cause this workflow to be executed and that's why it's needed because act is giving a container without node for some unexplained reason. but ALSO it's just easier to install node & docker & add the registry container and then run jest directly because you can easily debug the code and such.

.PHONY: test
test: run-docker-registry bootstrap
npm run build
./act -v -P ubuntu-latest=ghcr.io/catthehacker/ubuntu:js-latest -j test
Copy link
Contributor

Choose a reason for hiding this comment

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

I may have missed it — are we explicitly installing act somewhere in this setup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, in bootstrap. the python functional tests are running act locally AND when run on github. this is not ideal, but i didn't change this behavior. the new jest tests are being run directly from npm on the github runner

@luhring
Copy link
Contributor

luhring commented Aug 19, 2021

@kzantow Also, are we at a point now where it's worth adding additional status checks as required for this repo? (Currently the only required check is DCO.)

@kzantow
Copy link
Contributor Author

kzantow commented Aug 19, 2021

@kzantow Also, are we at a point now where it's worth adding additional status checks as required for this repo? (Currently the only required check is DCO.)

I think so, this would mean at least that tests don't go stale! :)

@luhring
Copy link
Contributor

luhring commented Aug 19, 2021

I think so, this would mean at least that tests don't go stale! :)

Okay awesome. Should I just make ALL of the checks showing up on this PR required?

@kzantow kzantow merged commit 5fd84ca into anchore:main Aug 19, 2021
@kzantow kzantow deleted the chore/upgrade-0.16.0 branch August 19, 2021 20:25
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.

Update Grype to a modern version
2 participants