Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

test: added test scripts for the functional tests #1430

Merged
kayrus merged 1 commit into from
Feb 24, 2016
Merged

test: added test scripts for the functional tests #1430

kayrus merged 1 commit into from
Feb 24, 2016

Conversation

@kayrus
Copy link
Contributor

@kayrus kayrus commented Feb 11, 2016

subj

@jonboulle
Copy link
Contributor

jonboulle commented Feb 11, 2016

I don't understand the use case for this. can you help me understand what it provides over just a bash script that runs this? Since we still need etcd, systemd etc running on the host.

@kayrus kayrus changed the title test: added test-docker script for functional tests [WIP] test: added test-docker script for functional tests Feb 11, 2016
@kayrus
Copy link
Contributor Author

kayrus commented Feb 11, 2016

It is still WIP. And this script will be executed on coreos.

@kayrus kayrus force-pushed the kayrus/docker_test branch from 5a8b858 to 27b9686 Compare February 11, 2016 18:02
@tixxdz
Copy link
Contributor

tixxdz commented Feb 12, 2016

You may also check if etcd is up.

@jonboulle
Copy link
Contributor

jonboulle commented Feb 12, 2016

What is the point of doing this in docker at all? Look how much stuff you necessarily need to bring in from the host..

@kayrus
Copy link
Contributor Author

kayrus commented Feb 12, 2016

@jonboulle this will allow you to run tests directly on coreos. which already has etcd.

@jonboulle
Copy link
Contributor

jonboulle commented Feb 12, 2016

Why do you need docker for that? The functional tests run fine on coreos
without it

kayrus notifications@github.com schrieb am Fr., 12. Feb. 2016 11:50:

@jonboulle https://github.com/jonboulle this will allow you to run
tests directly on coreos. which already has etcd.

—
Reply to this email directly or view it on GitHub
#1430 (comment).

@kayrus
Copy link
Contributor Author

kayrus commented Feb 12, 2016

@jonboulle there is no go binary inside coreos.

@antrik
Copy link
Contributor

antrik commented Feb 12, 2016

To elaborate a bit: according to functional/README.md, the official way to run tests is inside a CoreOS VM, typically through Vagrant. However, as CoreOS doesn't provide a Go environment itself, the README suggest mapping it from outside the VM using Vagrant shared folders. But that's neither easy nor a self-contained reproducible environment. In our understanding, the canonical way to get a Go environment inside CoreOS is by running a golang container.

@kayrus kayrus changed the title [WIP] test: added test-docker script for functional tests [WIP] test: added test scripts for the functional tests Feb 12, 2016
@kayrus kayrus mentioned this pull request Feb 15, 2016
Closed
@jonboulle
Copy link
Contributor

jonboulle commented Feb 16, 2016

@antrik thanks for the context, very helpful. I discussed out of band with @mirkoboehm and we'll circulate notes shortly

@kayrus kayrus force-pushed the kayrus/docker_test branch 6 times, most recently from 2d4ba84 to b67bfb8 Compare February 17, 2016 16:33
@kayrus kayrus mentioned this pull request Feb 18, 2016
Closed
@kayrus kayrus force-pushed the kayrus/docker_test branch from ce6756c to 9f9078f Compare February 18, 2016 16:20
@kayrus
Copy link
Contributor Author

kayrus commented Feb 18, 2016

@jonboulle tests passed. should we squash and merge?

@kayrus kayrus force-pushed the kayrus/docker_test branch 2 times, most recently from 83c56da to 112c46d Compare February 19, 2016 10:07
jonboulle
jonboulle reviewed Feb 19, 2016
View reviewed changes
functional/platform/nspawn.go Outdated

// minimum requirements for running systemd/coreos in a container
fmt.Sprintf("mkdir -p %s/usr", fsdir),
//fmt.Sprintf("mkdir -p %s/lib", fsdir), //ubuntu
Copy link
Contributor

@jonboulle jonboulle Feb 19, 2016

Choose a reason for hiding this comment

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

can you explain all these?

Copy link
Contributor Author

@kayrus kayrus Feb 19, 2016

Choose a reason for hiding this comment

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

I tried to run these tests in ubuntu without cloud-config. Decided to keep this. If you don't like - i'll remove.

Copy link
Contributor

@jonboulle jonboulle Feb 19, 2016

Choose a reason for hiding this comment

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

We definitely shouldn't leave commented lines in here. Are the lines totally superfluous now? The tests all work fine without?

Copy link
Contributor Author

@kayrus kayrus Feb 19, 2016

Choose a reason for hiding this comment

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

Yes, they do. Will remove ubuntu stuff. But will keep .. 500 ... for future support.

@reneengelhard reneengelhard mentioned this pull request Feb 19, 2016
Closed
jonboulle
jonboulle reviewed Feb 19, 2016
View reviewed changes
build
@@ -1,41 +1,45 @@
#!/bin/bash -e

CDIR=$(cd `dirname $0` && pwd)
Copy link
Contributor

@jonboulle jonboulle Feb 19, 2016

Choose a reason for hiding this comment

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

is this working around some problem you encountered?

Copy link
Contributor Author

@kayrus kayrus Feb 19, 2016

Choose a reason for hiding this comment

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

Yes, when you run scripts from home directory for example. cd ~ ; ./fleet/build

jonboulle
jonboulle reviewed Feb 19, 2016
View reviewed changes
functional/README.md Outdated
Then run the functional tests:

```sh
$ git clone https://github.com/coreos/fleet
Copy link
Contributor

@jonboulle jonboulle Feb 19, 2016

Choose a reason for hiding this comment

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

haven't we already cloned?

@kayrus kayrus force-pushed the kayrus/docker_test branch from 97daef8 to 79a60c7 Compare February 19, 2016 14:29
tixxdz
tixxdz reviewed Feb 19, 2016
View reviewed changes
functional/test Outdated
eval $(ssh-agent)
fi

chmod 600 functional/fixtures/id_rsa
Copy link
Contributor

@tixxdz tixxdz Feb 19, 2016

Choose a reason for hiding this comment

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

Please document why we need this one.

Copy link
Contributor Author

@kayrus kayrus Feb 19, 2016

Choose a reason for hiding this comment

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

Done

@jonboulle
Copy link
Contributor

jonboulle commented Feb 22, 2016

For the sake of moving forward:
i) squash commit history into something sensible
ii) merge this
iii) please follow up the comments I've just added

@kayrus kayrus force-pushed the kayrus/docker_test branch from 7ece6d7 to 180b7e7 Compare February 24, 2016 14:49
kayrus added a commit that referenced this pull request Feb 24, 2016
@kayrus
Merge pull request #1430 from endocode/kayrus/docker_test
8d7930c 
test: added test scripts for the functional tests
@kayrus kayrus merged commit 8d7930c into coreos:master Feb 24, 2016
@kayrus kayrus deleted the kayrus/docker_test branch March 30, 2016 07:47
@kayrus kayrus changed the title [WIP] test: added test scripts for the functional tests test: added test scripts for the functional tests Jun 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kayrus @jonboulle @tixxdz @antrik @reneengelhard