Add integration tests

[FR4NK-W]

Add integration tests

Fixes #126 for helloworld and bwtester

---

This change is 

[juagargi]

Reviewed 12 of 17 files at r2.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @FR4NK-W and @matzf)

[FR4NK-W]

Reviewable status: 10 of 12 files reviewed, 9 unresolved discussions (waiting on @juagargi and @matzf)

---

Makefile, line 14 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Thanks

Done.

---

Makefile, line 24 at r1 (raw file):

Is there a technical reason this is needed?

No, it is not even a GNU standard target https://www.gnu.org/prep/standards/html_node/Standard-Targets.html

enforcing to run the linter was kind of the point

The linter is run for all targets except for this build target (and this build target is not reused anywhere else, all others use all as a prerequisite).
Having this target makes it easier to do intermediate builds, to test things (the linter is sometimes too strict, and you don't know yet what you can / want to exclude).

---

Makefile, line 47 at r1 (raw file):

explicitly list each individual target instead of this macro madness.

I went with go test -v -tags=integration ./... as you suggested.

And opened an issue (#144) for the macro thing in the build target: it is also wrong, it build multiple binaries with the same name (server, client), which then overwrite each other when installed.
The build target should build binaries with the same names as those used by the packaged version.

---

.circleci/config.yml, line 49 at r1 (raw file):

Previously, FR4NK-W wrote…

Cloning everything was slow. You are right, from the replace directive we might get a commit that is not on the master branch

Done.

---

.circleci/config.yml, line 91 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

All of them.

Done.

---

.circleci/config.yml, line 79 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

There is topology/Tiny4.topo.

Good point, didn't know about that one and it's exactly what we want here.
Still need to look into local topo IPv6 at some point.

---

.circleci/config.yml, line 101 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

This is no longer needed, correct?

Correct

---

_examples/helloworld/helloworld_integration_test.go, line 92 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

This library here: https://github.com/scionproto/scion/tree/master/go/lib/integration
Used e.g. here: https://github.com/scionproto/scion/blob/master/go/tools/scmp/scmp_integration/main.go

Done.

---

_examples/helloworld/helloworld_integration_test.go, line 48 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

Could the framework just call these matchers on the full output instead? That would allow to get rid of the bool parameter, which I find a bit surprising.
Note, perhaps, that the golang regexp has a multi-line mode so that matches for a full line are still easily possible -- it's enabled with a (?m) in the regexp string, e.g.:

`(?m)^foo$`

Then the output processing go routine would need to buffer the full output, which kind of defeats the point of using a Scanner.

Also having this simple boolean parameter allows matching functions to keep some state, to potentially match differently if they already had a match or not.
At least that was the idea.

[matzf]

Reviewed 2 of 2 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @FR4NK-W)

---

Makefile, line 24 at r1 (raw file):

Previously, FR4NK-W wrote…

Is there a technical reason this is needed?

No, it is not even a GNU standard target https://www.gnu.org/prep/standards/html_node/Standard-Targets.html

enforcing to run the linter was kind of the point

The linter is run for all targets except for this build target (and this build target is not reused anywhere else, all others use all as a prerequisite).
Having this target makes it easier to do intermediate builds, to test things (the linter is sometimes too strict, and you don't know yet what you can / want to exclude).

Ok, then lets change all to all: lint build

---

_examples/helloworld/helloworld_integration_test.go, line 48 at r2 (raw file):

Previously, FR4NK-W wrote…

Then the output processing go routine would need to buffer the full output, which kind of defeats the point of using a Scanner.

Also having this simple boolean parameter allows matching functions to keep some state, to potentially match differently if they already had a match or not.
At least that was the idea.

Ok, fair enough.
The regexp matcher could also be applied on stream, but I guess then the interface would become more complicated rather than simpler.
Note that this type of state can easily be covered by a (multi line) regexp.

---

camerapp/imageserver/logo.jpg, line 0 at r3 (raw file):
Left over file? Does not seem to be used right now. Fwiw, there is a dog.jpg already lying around in the http example server.

[FR4NK-W]

Reviewable status: 8 of 10 files reviewed, 2 unresolved discussions (waiting on @matzf)

---

Makefile, line 24 at r1 (raw file):

Previously, matzf (Matthias Frei) wrote…

Ok, then lets change all to all: lint build

Done.

---

camerapp/imageserver/logo.jpg, line at r3 (raw file):

Previously, matzf (Matthias Frei) wrote…

Left over file? Does not seem to be used right now. Fwiw, there is a dog.jpg already lying around in the http example server.

Done.

[matzf]

Reviewed 3 of 4 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved