Skip to content

Conversation

@cgwalters
Copy link
Member

In continuing work on external tests, passing -E gets awkward
for a few reasons. First, it ties the tests too heavily to the
source directory.

This supports a model like https://wiki.gnome.org/Initiatives/GnomeGoals/InstalledTests
and Debian autopkg test etc. where the tests are installed.

This ensures a clean separation from the source directory,
which also helps us in e.g. supporting built binaries as is done
in ostreedev/ostree#2048

For example, one thing we can do after this is extend the
coreos-assembler container with the results of make install-tests
from relevant git repositories (or make RPMs of them).

In continuing work on external tests, passing `-E` gets awkward
for a few reasons.  First, it ties the tests too heavily to the
source directory.

This supports a model like https://wiki.gnome.org/Initiatives/GnomeGoals/InstalledTests
and Debian autopkg test etc. where the tests are installed.

This ensures a clean separation from the source directory,
which also helps us in e.g. supporting built binaries as is done
in ostreedev/ostree#2048

For example, one thing we can do after this is extend the
coreos-assembler container with the results of `make install-tests`
from relevant git repositories (or make RPMs of them).
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

// can be placed; for example, a project like ostree can install
// tests at /usr/lib/coreos-assembler/tests/kola/ostree/...
// and this will be automatically picked up.
const InstalledTestsDir = "/usr/lib/coreos-assembler/tests/kola"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could we make this a var instead, e.g.

const defaultInstalledTestDirs = "/usr/lib/coreos-assembler/tests/kola"
var InstalledTestsDir = defaultInstalledTestDir

Such that go run -ldflags '-X kola.harness.InstalledTestsDir="foo" ...' works?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is that this is a standard directory for other components to install to; we already have a way to do "find tests from elsewhere" via -E right?

Copy link
Contributor

@darkmuggle darkmuggle left a comment

Choose a reason for hiding this comment

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

Completely optional nit.

/lgtm

@cgwalters
Copy link
Member Author

/refresh

@cgwalters cgwalters added the lgtm label May 13, 2020
@openshift-merge-robot openshift-merge-robot merged commit df6fa87 into coreos:master May 13, 2020
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 14, 2020
This builds on
coreos/coreos-assembler#1441
to install our tests rather than running them from the source
directory.  This model will more cleanly allow us to ship
our tests along with a test container or elsewhere, separate
from the source directory.

Also prep for ostreedev#2048
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 15, 2020
This builds on
coreos/coreos-assembler#1441
to install our tests rather than running them from the source
directory.  This model will more cleanly allow us to ship
our tests along with a test container or elsewhere, separate
from the source directory.

Also prep for ostreedev#2048
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 15, 2020
This builds on
coreos/coreos-assembler#1441
to install our tests rather than running them from the source
directory.  This model will more cleanly allow us to ship
our tests along with a test container or elsewhere, separate
from the source directory.

Also prep for ostreedev#2048
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 15, 2020
This builds on
coreos/coreos-assembler#1441
to install our tests rather than running them from the source
directory.  This model will more cleanly allow us to ship
our tests along with a test container or elsewhere, separate
from the source directory.

Also prep for ostreedev#2048
cgwalters added a commit to cgwalters/rpm-ostree that referenced this pull request May 23, 2020
Switch to the "installed" model introduced by:
coreos/coreos-assembler#1441

It's hard to support running tests *both* from the srcdir
and installed; in this case because we have a symlink that needs
to be followed, which kola knows how to do from the srcdir
but not when installed.  Let's establish a new convention of
`tests/kolainst`.   In our case we follow the symlink manually
for now.

That bit will be cleaned up when we eventually switch entirely
to kola tests.
cgwalters added a commit to cgwalters/rpm-ostree that referenced this pull request May 23, 2020
Switch to the "installed" model introduced by:
coreos/coreos-assembler#1441

It's hard to support running tests *both* from the srcdir
and installed; in this case because we have a symlink that needs
to be followed, which kola knows how to do from the srcdir
but not when installed.  Let's establish a new convention of
`tests/kolainst`.   In our case we follow the symlink manually
for now.

That bit will be cleaned up when we eventually switch entirely
to kola tests.
cgwalters added a commit to cgwalters/rpm-ostree that referenced this pull request May 26, 2020
Switch to the "installed" model introduced by:
coreos/coreos-assembler#1441

It's hard to support running tests *both* from the srcdir
and installed; in this case because we have a symlink that needs
to be followed, which kola knows how to do from the srcdir
but not when installed.  Let's establish a new convention of
`tests/kolainst`.   In our case we follow the symlink manually
for now.

That bit will be cleaned up when we eventually switch entirely
to kola tests.
openshift-merge-robot pushed a commit to coreos/rpm-ostree that referenced this pull request May 26, 2020
Switch to the "installed" model introduced by:
coreos/coreos-assembler#1441

It's hard to support running tests *both* from the srcdir
and installed; in this case because we have a symlink that needs
to be followed, which kola knows how to do from the srcdir
but not when installed.  Let's establish a new convention of
`tests/kolainst`.   In our case we follow the symlink manually
for now.

That bit will be cleaned up when we eventually switch entirely
to kola tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants