Skip to content

Add basic tests for the test caching functionality#20

Merged
rl-gitpod merged 6 commits into
mainfrom
rl/add-image-tests-2
Jun 9, 2021
Merged

Add basic tests for the test caching functionality#20
rl-gitpod merged 6 commits into
mainfrom
rl/add-image-tests-2

Conversation

@rl-gitpod
Copy link
Copy Markdown
Contributor

Also some minor tidy up.

Note that there is some odd behaviour with main (& this branch) atm.

If you run ./example.sh multiple times it is repeating tests incrementally down to 0. Possibly timestamp related, but I'll take a look post-dinner (or tomorrow)

I'll also look at expanding the cache test functionality i.e. changing a test should trigger a re-test, etc.

@rl-gitpod rl-gitpod requested a review from csweichel June 9, 2021 07:55
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 9, 2021

Codecov Report

Merging #20 (bc49af4) into main (69e9e5b) will increase coverage by 5.15%.
The diff coverage is 6.06%.

❗ Current head bc49af4 differs from pull request most recent head ec593c6. Consider uploading reports for the commit ec593c6 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            main      #20      +/-   ##
=========================================
+ Coverage   6.98%   12.13%   +5.15%     
=========================================
  Files          4        4              
  Lines        859      865       +6     
=========================================
+ Hits          60      105      +45     
+ Misses       792      741      -51     
- Partials       7       19      +12     
Impacted Files Coverage Δ
pkg/dazzle/combiner.go 0.00% <0.00%> (ø)
pkg/dazzle/build.go 4.52% <4.00%> (+4.52%) ⬆️
pkg/dazzle/registry.go 5.35% <14.28%> (+5.35%) ⬆️
pkg/dazzle/project.go 40.79% <0.00%> (+10.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69e9e5b...ec593c6. Read the comment docs.

Copy link
Copy Markdown
Collaborator

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

Nice one! LGTM

Comment thread pkg/dazzle/build.go Outdated
chunks map[string]*ociv1.Manifest
}

type removeOpts struct {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type removeOpts struct {
type removeBaseLayerOpts struct {

Comment thread pkg/dazzle/registry.go
)

// Registry provides container registry services
type Registry interface {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice one. We could consider (in a next step after this PR) to mock this interface proper, and make removeBaseLayer use this interface, too.

Copy link
Copy Markdown
Contributor Author

@rl-gitpod rl-gitpod Jun 9, 2021

Choose a reason for hiding this comment

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

next step

+1

mock this interface proper

mock or fake?

make removeBaseLayer use this interface

I was going to look at expanding the testing and it would make sense to take a look at doing it then.

@rl-gitpod rl-gitpod merged commit 943db41 into main Jun 9, 2021
@rl-gitpod rl-gitpod deleted the rl/add-image-tests-2 branch June 9, 2021 10:04
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.

3 participants