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

chore: track script to make local test images #476

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

willmurphyscode
Copy link
Contributor

Can be used for local development. Might be worth integrating into npm commands.

Remembering to start the local registry before running tests locally is cumbersome. Here's a shell script, copied essential from the docs, to do it for us.

Questions:

  1. Should this script be instead moved to an npm target?
  2. Should this script be extracted from test setup into its own task?
  3. How should we handle failure modes, e.g. where there's already a container named registry or a port is in use?
  4. Should we totally dispense with the local registry and just push test images to a container registry somewhere?

PR is a conversation starter, in other words. But this script has been kicking around locally for a while and I use it, so I wanted to share.

@wagoodman
Copy link
Contributor

Should we totally dispense with the local registry and just push test images to a container registry somewhere?

this would require that local development requires non local resources for testing, which isn't ideal

Should this script be instead moved to an npm target?

yes -- I think the idiomatic way to deal with stuff like this is as a new script entry here

"scripts": {

Should this script be extracted from test setup into its own task?

can it be both? That is, have the test automatically set it up if its not already running, or use an already running one?

How should we handle failure modes, e.g. where there's already a container named registry or a port is in use?

My vote would be to name it sbom-action-registry with a unique-enough static port (so probably not 5000) and always use it (if already created then use that)

@kzantow
Copy link
Contributor

kzantow commented Jul 8, 2024

Questions:

Should this script be instead moved to an npm target?

This should be integrated into the tests so running npm test does any required setup. This can be done with before/after hooks: https://jestjs.io/docs/setup-teardown#one-time-setup

Should this script be extracted from test setup into its own task?

No, it should be integrated with the tests.

How should we handle failure modes, e.g. where there's already a container named registry or a port is in use?

Fail the tests.

Should we totally dispense with the local registry and just push test images to a container registry somewhere?

This would be a fine solution, too -- it could be a GHCR package linked to this repo.

@willmurphyscode
Copy link
Contributor Author

It sounds like consensus is to move startup code from

- name: Build images
run: |
for distro in alpine centos debian; do
docker build -t localhost:5000/match-coverage/$distro ./tests/fixtures/image-$distro-match-coverage
docker push localhost:5000/match-coverage/${distro}:latest
done

to a target in

"scripts": {

It might be nice to go further and make npm test call the new functionality, but that has a bit of difficulty:

  1. A developer wants to run npm test on every code change
  2. A developer needs to run npm run start-test-registry (or whatever we call it) only if the images change / once per session, which is way less often than item 1.

Therefore, for npm test to also start the test registry, we'd need to make starting the test registry idempotent, which in turn would be annoying because then someone would have to restart the test registry if they changed an image file.

I'll see how hard it is to make an npm run start-test-registry command idempotent, and failing that at least add it as a script and document it.

@willmurphyscode willmurphyscode self-assigned this Jul 8, 2024
@kzantow
Copy link
Contributor

kzantow commented Jul 8, 2024

These images are not really ever changing, the main purpose was just to validate that the command is actually running and generating files and that the output isn't changing drastically. I strongly favor just pushing these images to GHCR and updating the tests to point to them.

Can be used for local development. Might be worth integrating into npm commands.

Signed-off-by: Will Murphy <will.murphy@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants