Add integration test coverage for adding an app in a GitLab repo #1101
Conversation
jpellizzari
force-pushed
the
jp-gitlab-support
branch
3 times, most recently
from
e2ca5bd
to
4d7c523
Compare
jpellizzari
added
type/enhancement
jpellizzari
force-pushed
the
jp-gitlab-support
branch
from
e3bef64
to
74178ee
Compare
jpellizzari
changed the title
| @@ -58,7 +58,7 @@ func CreateRepo(ctx context.Context, gp gitprovider.Client, url string) (gitprov | |||
| return nil, nil, fmt.Errorf("could not reconcile org repo: %w", err) | |||
| } | |||
|
|
|||
| err = utils.WaitUntil(os.Stdout, 3*time.Second, 9*time.Second, func() error { | |||
| err = utils.WaitUntil(bytes.NewBuffer([]byte{}), 3*time.Second, 9*time.Second, func() error { | |||
There was a problem hiding this comment.
I'm curious on why came back to this. Last time, we agreed to print to stdout to have better debug logs. Any change of mind?
There was a problem hiding this comment.
Ah yes I should have called this out.
Logging to stdout during the retry loop populates the output with unnecessary information. We expect the retry to fire a couple of times, so we don't need to tell the user about it.
It ensures that we only log things when they have gone wrong. If the WaitUntil function hides important output or doesn't return an error at the right time, there is a bug with WaitUntil.
There was a problem hiding this comment.
I see, are you suggesting printing out all errors only if the last attempt fails?
There was a problem hiding this comment.
I see, are you suggesting printing out all errors only if the last attempt fails?
I am suggesting only printing the error if waitUntil returns one.
jpellizzari
force-pushed
the
jp-gitlab-support
branch
from
37134d9
to
784456c
Compare
Adds some much needed test coverage for GitLab. Only covers the Merge Request case (not auto-merge).
These tests have reached a tipping point IMO and need to be refactored into a table-style test to reduce some of the repetition. I will handle that in another PR, as getting the coverage is more important than making them DRY.