Extract testenv #355
Extract testenv #355
Conversation
kyma-bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
kyma-bot
added
the
size/L
dbadura
force-pushed
the
extract-testenv
branch
from
8eb82ab
to
3d8f379
Compare
dbadura
requested review from
m00g3n,
pPrecel,
kwiatekus,
Cortey,
anoipm and
MichalKalke
as code owners
kyma-bot
removed
the
do-not-merge/work-in-progress
| "testing" | ||
| ) | ||
|
|
||
| func SetUpTestEnv(t *testing.T) (cl client.Client, env *envtest.Environment) { |
There was a problem hiding this comment.
The name of the package contains info about the destination of the content. We don't need to keep the same info in the func name. I would suggest renaming the SetUpTestEnv to just SetUp or something like Configure.
There was a problem hiding this comment.
The testenv_test.SetUpTestEnv(t) looks a bit strange 😅 That's why I suggested changing it into testenv_test.SetUp(t) or, with propositions from the comment below, even into testenv.SetUp(t)/env_test.SetUp(t).
There was a problem hiding this comment.
I propose names Start and Stop. Those are similar to standard envtest package methods
| @@ -0,0 +1,54 @@ | |||
| package testenv_test | |||
There was a problem hiding this comment.
I wonder if the testenv_test keeps redundant info. The _test suffix is used to define the desire of the package, right? I would suggest renaming it into env_test.
To be honest, I think the testenv keeps all important information about the content under it, so maybe we even should remove this _test suffix.
But if we don't want to allow importing public methods from this pkg in a non-test file we should add the _test suffix to the file name.
There was a problem hiding this comment.
It's not possible to use test file package in other tests I renamed the file to just envtest.
I removed the additional test from package as Start and Stop accept testing.T which indicate that those methods should be used in test only,
|
/test all |
Description
Changes proposed in this pull request:
Related issue(s)
kyma-project/kyma#17562