Skip to content

chore: add unit tests for Prune function#46

Merged
mdelapenya merged 5 commits intotestcontainers:mainfrom
mdelapenya:unit-tests
Nov 17, 2022
Merged

chore: add unit tests for Prune function#46
mdelapenya merged 5 commits intotestcontainers:mainfrom
mdelapenya:unit-tests

Conversation

@mdelapenya
Copy link
Member

@mdelapenya mdelapenya commented Nov 7, 2022

What does this PR do?

It adds unit tests for the Prune function and for that:

  • we have extracted the log entry printing the length of the arrays including the pruned resources (containers, networks, volumes and images) to the main function, therefore the prune function has to return those lengths. This way it's possible to test the function in isolation without calling the main function.
  • we have added a dependency with testcontainers-go, to easily create containers and networks in the tests, in order to check if they are effectively removed by the prune function. For that, we are creating resources with a test label, which is used to build the deathNode payload to be sent to the prune function. This deathNote represents a JSON filter to be used by Docker to retrieve the specific resource (containers, networks, volumes and images)

For the images, I see that this block is not deterministic at the test level: at the moment 25 images are created using the Docker API, the images prune report includes not only the deleted images but untagged ones.

for _, image := range imagesPruneReport.ImagesDeleted {
	deletedImagesMap[image.Deleted] = true
}

The prune report is formed by a struct with two fields: Deleted and Untagged. When a layer is deleted the untagged field is empty, and viceversa. The test for the images is currently failing because it's not deterministic and dependent on the number of instructions in the Dockerfile used to build the image at the test level. In this test, we are adding a very simple Dockerfile extending the nginx:alpine base image.

See GH action:

--- FAIL: TestPrune (29.31s)
    --- FAIL: TestPrune/Death_note_removing_images (7.16s)
        main_test.go:263: 
            	Error Trace:	/home/runner/work/moby-ryuk/moby-ryuk/main_test.go:263
            	Error:      	Not equal: 
            	            	expected: 0
            	            	actual  : 3  <-- 3 containers removed??
            	Test:       	TestPrune/Death_note_removing_images
        main_test.go:266: 
            	Error Trace:	/home/runner/work/moby-ryuk/moby-ryuk/main_test.go:266
            	Error:      	Not equal: 
            	            	expected: 25
            	            	actual  : 39  <-- 39 images removed??
            	Test:       	TestPrune/Death_note_removing_images

A simple solution for the test would be to check that the length of the deletedImages is greater than zero, but I wanted to check with you first before adding that condition. At the same hand, I advocate for a change at the moment we increase the number of deleted images: instead of appending an entry to the map for each element in the pruneReport.ImagesDeleted prune report (see above block), I'd ask for those entries where the Untagged field is not empty (capturing the real image that was deleted)

Why is important?

Unit test the prune code 😅

@mdelapenya mdelapenya added the enhancement New feature or request label Nov 7, 2022
@mdelapenya mdelapenya self-assigned this Nov 7, 2022
@mdelapenya mdelapenya marked this pull request as ready for review November 9, 2022 11:56
@mdelapenya mdelapenya merged commit f21e6a5 into testcontainers:main Nov 17, 2022
@mdelapenya mdelapenya deleted the unit-tests branch November 28, 2022 13:34
mdelapenya added a commit to gesellix/moby-ryuk that referenced this pull request Dec 1, 2022
* main:
  feat: add golangci-lint (testcontainers#48)
  chore: add unit tests for Prune function (testcontainers#46)
  chore(ci): only push Docker image on tags (testcontainers#47)
  chore: bump build system versions (testcontainers#45)
  chore: rename branch to main (testcontainers#44)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants