Replace print with logger in image_cache.go, fixes formatting

[harshalmittal4]

This attempts to fix #880 by using a logger

Hey @jjbustamante, @natalieparellano,
For testing, the invocation of logger.Warnf() in image_cache.go requires c.DeleteOrigImage() to return a non-nil error. I tried simulating it using a spy for ImageCache and mock method DeleteOrigImage() that returns an error but it did not help -

when("attempting to delete original image gives non nil error", func() {
	it.Before(func() {
		fakeOriginalImage = fakes.NewImage("fake-image", "", local.IDIdentifier{ImageID: "fakeOrigImage"})
		fakeNewImage = fakes.NewImage("fake-image", "", local.IDIdentifier{ImageID: "fakeNewImage"})
	})
	it("should log error as warning", func() {
		spyErrImageCache := &SpyErrImageCache{origImage: fakeOriginalImage, newImage: fakeNewImage, logger: cmd.DefaultLogger,}
		err := spyErrImageCache.Commit()
		h.AssertNil(t, err)
		h.AssertEq(t, fakeOriginalImage.Found(), false)
	})
})
type SpyErrImageCache struct{
	committed bool
	origImage imgutil.Image
	newImage  imgutil.Image
	logger    log.Logger
}

func (c *SpyErrImageCache) DeleteOrigImage() error {
	return errors.New("error deleting original image")
}

Could you please suggest how can it be tested and review other changes

[jjbustamante]

Hi @harshalmittal4 thanks for working on this!

Taking a look, I think maybe you can try mocking the origImage imgutil.Image. You need the c.DeleteOrigImage() to throw an error to test your code. Something like:

type fakeErrorImage struct {
	imgutil.Image
}

func newFakeImageErrIdentifier(origImage imgutil.Image) *fakeErrorImage {
	return &fakeErrorImage{Image: origImage}
}

func (f *fakeErrorImage) Identifier() (imgutil.Identifier, error) {
	return nil, errors.New("error deleting original image")
}

Then in your test case:

var fakeErrorImage *fakeErrorImage
it.Before(func() {
	fakeOriginalImage = fakes.NewImage("fake-image", "", local.IDIdentifier{ImageID: "fakeOrigImage"})
       // wrap your fake image into the one that throws the error you need
	fakeErrorImage = newFakeImageErrIdentifier(fakeOriginalImage)
       // ... more code
})

That should help you to go into your warning message, also provide a mock logger to record the method was called and you can do some assert in the test

[natalieparellano]

@harshalmittal4 this looks good! I think we also need to update the function call here:

lifecycle/acceptance/exporter_test.go

Line 259 in 3be3585

subject, err := cache.NewImageCacheFromName(cacheImageName, authn.DefaultKeychain)

[harshalmittal4]

Thanks a lot @jjbustamante , @natalieparellano ! I've updated the PR including both the comments, please take a look.

[harshalmittal4]

Unrelated to this, can you also please take a look at this issue when running acceptance tests -
I am using mac M1 (arm64) and getting the following error on running command make test-

> Running acceptance tests...
go test  -v -count=1 -tags=acceptance -timeout=2400s ./acceptance/...
=== RUN   TestVersion
    binaries.go:28: Building binaries: [make build-darwin-arm64]
    binaries.go:29: Expected nil: failed to execute command: [make build-darwin-arm64], exit status 2, make[1]: *** No rule to make target `build-darwin-arm64'.  Stop.
    .
    .
    === RUN   TestRestorer
    restorer_test.go:35: Restorer acceptance tests are not yet supported on non-amd64
--- SKIP: TestRestorer (0.00s)

So is there a way to run these on M1 mac?

[jabrown85]

This looks good!

As far as running the acceptance tests on m1 - I have not tried it myself as I don't yet have an arm machine. Maybe someone else can advise.

[jjbustamante]

Awesome job @harshalmittal4! Thanks a lot for helping us on this. ❤️

[harshalmittal4]

Thanks much @jjbustamante , @natalieparellano for all the help! I'm an open-source beginner and really glad to be able to contribute 😃

[natalieparellano]

Awesome work @harshalmittal4!