Magic catalog #2527
Magic catalog #2527
Conversation
perdasilva
force-pushed
the
magic_catalog
branch
3 times, most recently
from
85cc57f
to
b639cab
Compare
|
/assign @kevinrizza |
perdasilva
force-pushed
the
magic_catalog
branch
from
b639cab
to
6465f47
Compare
perdasilva
force-pushed
the
magic_catalog
branch
from
6465f47
to
615dcf0
Compare
openshift-ci
bot
added
the
needs-rebase
Signed-off-by: Per G. da Silva <perdasilva@redhat.com>
perdasilva
force-pushed
the
magic_catalog
branch
from
615dcf0
to
bdf7e7e
Compare
openshift-ci
bot
removed
the
needs-rebase
There was a problem hiding this comment.
Wow, thanks for putting this together! It should really help.
/approve
(reminder: we need to add @perdasilva to the approver set)
| func (c *magicCatalog) makeCatalogSourcePod() *corev1.Pod { | ||
|
|
||
| const ( | ||
| image = "quay.io/operator-framework/upstream-opm-builder" |
There was a problem hiding this comment.
I think this might need to be injectable for downstream CI at some point.
There was a problem hiding this comment.
that's a very good point - we could think of using env vars or something like that
| TeardownNamespace(generatedNamespace.GetName()) | ||
| }) | ||
|
|
||
| It("Deploys and Undeploys a File-based Catalog", func() { |
| const catalogName = "test" | ||
| namespace := generatedNamespace.GetName() | ||
| kubeClient := ctx.Ctx().Client() | ||
| provider, err := NewFileBasedFiledBasedCatalogProvider("../test/e2e/testdata/fbc_catalog.json") |
There was a problem hiding this comment.
Personally, I'd rather see the FBC JSON literal in the test case itself than in a golden file, but this works for now.
There was a problem hiding this comment.
tbh - I'm not a huge fan of big string literals, especially when we can't nicely format them due to code formatting or indentation, etc. For small stuff sure, for bigger files it's start to get awkward for me. But this is a taste thing. I've tried to create an abstraction through this provider interface where we can plug-in different fbc sources (file, string, etc.). When I have a little more time, I'd like to have a way to create the fbcs programatically. Might be even more readable that way...idk...
openshift-ci
bot
added
the
approved
There was a problem hiding this comment.
I think there's a good chunk of potential linting errors (the golangci-lint work we just merged excludes the test/e2e package for now, but we'll remove that exclusion over time) that the sanity check isn't complaining about, so I'm not super concerned. I don't have any problem with the implementation, and we should even have an issue open for switching to FBC for test fixtures open somewhere. Thanks for renaming that test utilities package, I've had the urge to rename that for a while now.
/lgtm
| Name: c.configMapName, | ||
| Namespace: c.namespace, | ||
| }, | ||
| Immutable: &isImmutable, |
There was a problem hiding this comment.
Super unimportant: we likely have a test helper function (or at least we should) that returns a pointer to a bool somewhere.
| } | ||
|
|
||
| func (c *magicCatalog) UndeployCatalog(ctx context.Context) []error { | ||
| var errs []error = nil |
There was a problem hiding this comment.
nit: this appears to be redundant as var errs []error should be equivalent to an empty slice.
There was a problem hiding this comment.
right! old habits hehehe
| func waitFor(fn func() (bool, error)) error { | ||
| return wait.Poll(pollInterval, pollDuration, func() (bool, error) { | ||
| return fn() | ||
| }) | ||
| } |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: njhale, perdasilva, timflannagan 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 |
Description of the change:
This PR instroduces
MagicCatalog, an easy(er) way to deploy custom/arbitrary catalogs for e2e tests. I've also refactored theutils_test.gotoutils.goand started taking the first steps to cut the utilities out into its own package, if possible.At the moment it only includes a single implementation of the
FileBasedCatalogProvider, which serves up the FBC given by a filesystem path (theFileBasedFileBasedCatalogProvider). Other providers could be imagined that give a more programmatic approach to catalog building...This PR could probably be improved by bringing in registry classes (maybe the FileBasedCatalogProvider could provide some FileBasedCatalog obj, etc.). Curious to know what you guys think.
Motivation for the change:
It's really hard to bring arbitrary
Reviewer Checklist
/doc