test for func run with digested image override #2650
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: David Fridrich <[email protected]>
knative-prow
bot
added
the
approved
knative-prow
bot
added
the
size/M
gauron99
requested review from
matejvasek and
lkingland
and removed request for
dsimansk and
vyasgun
gauron99
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2650 +/- ##
==========================================
+ Coverage 64.13% 65.42% +1.28%
==========================================
Files 130 130
Lines 15518 15518
==========================================
+ Hits 9953 10152 +199
+ Misses 4625 4389 -236
- Partials 940 977 +37
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: David Fridrich <[email protected]>
matejvasek
reviewed
Jan 16, 2025
matejvasek
reviewed
Jan 16, 2025
matejvasek
reviewed
Jan 16, 2025
| ctx, cancel := context.WithCancel(context.Background()) | ||
| runErrCh := make(chan error, 1) | ||
| go func() { | ||
| _, err := cmd.ExecuteContextC(ctx) |
There was a problem hiding this comment.
This probably could be just err := cmd.ExecuteContext(ctx)
matejvasek
reviewed
Jan 16, 2025
| // run function with above argument | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| runErrCh := make(chan error, 1) | ||
| go func() { |
There was a problem hiding this comment.
I think we could get rid of this asynchronicity by closing err channel in the runner:
diff --git a/cmd/run_test.go b/cmd/run_test.go
index 7340ca59f..88130ea83 100644
--- a/cmd/run_test.go
+++ b/cmd/run_test.go
@@ -398,7 +398,8 @@ func TestRun_DirectOverride(t *testing.T) {
if f.Build.Image != overrideImage {
return nil, fmt.Errorf("Expected image to be overridden with '%v' but got: '%v'", overrideImage, f.Build.Image)
}
- errs := make(chan error, 1)
+ errs := make(chan error)
+ close(errs)
stop := func() error { return nil }
return fn.NewJob(f, "127.0.0.1", "8080", errs, stop, false)
}
@@ -440,25 +441,16 @@ func TestRun_DirectOverride(t *testing.T) {
// run function with above argument
ctx, cancel := context.WithCancel(context.Background())
- runErrCh := make(chan error, 1)
- go func() {
- _, err := cmd.ExecuteContextC(ctx)
+ defer cancel()
+
+ err = cmd.ExecuteContext(ctx)
if err != nil {
- runErrCh <- err // error was not expected
- return
+ t.Fatal(err)
}
// Ensure invocation doesnt happen for the second time as the image was
// provided with a digest (should not build)
if builder2.BuildInvoked {
- runErrCh <- fmt.Errorf("Function was not expected to build again but it did")
- }
-
- close(runErrCh) // release the waiting parent process
- }()
- cancel() // trigger the return of cmd.ExecuteContextC in the routine
- <-ctx.Done()
- if err := <-runErrCh; err != nil { // wait for completion of assertions
- t.Fatal(err)
+ t.Fatal("Function was not expected to build again but it did")
}
}
|
Some nits, but otherwise looks good. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gauron99, matejvasek The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add a test for "Func run with digested image when function is already built, expecting to override said built image"