Isolated integration tests #149
Conversation
lupine
force-pushed
the
nt-isolated-integration-tests
branch
5 times, most recently
from
63721db to
254a7f9
Compare
|
❤️ Haven't forgotten about the other PRs, just starting a new job so focused elsewhere at the moment :) Will take a look when I can. This looks awesome though, getting these running in CI was a good next step before expanding them to cover more cases, nice job :) |
lupine
force-pushed
the
nt-isolated-integration-tests
branch
from
254a7f9 to
c808c43
Compare
|
No worries; I'm just fiddling as I have time 😅 . The PR only exists at the moment because it's required to get the CI running. Best of luck with the new job! |
lupine
force-pushed
the
nt-isolated-integration-tests
branch
5 times, most recently
from
5c0dd89 to
0b9bb24
Compare
| uses: actions-rs/cargo@v1 | ||
| with: | ||
| command: test | ||
| args: --features=mocks | ||
|
|
||
| - name: Start Mosquitto |
There was a problem hiding this comment.
Not really sure why the docker run approach isn't working inside github actions, and it's painful to debug.
There was a problem hiding this comment.
I think this is fine; it's like sharing a postgres database, which is common in tests.
It's occurred to me that I could achieve multitenancy (and even test parallelisation) by applying a custom MQTT prefix (i.e. topics would be lxp_test_XXXX/...) in lxp-bridge's config. Will look into that.
| serial: 5555555555 | ||
| datalog: 2222222222 | ||
| heartbeats: true | ||
| # FIXME: get the specs passing with this enabled |
There was a problem hiding this comment.
This isn't actually in master yet.
Since we're now running an lxp-bridge per spec, we can vary the config-file per spec, rather than having it hardcoded. That's a few revisions away though.
| @@ -1,27 +0,0 @@ | |||
| #! /usr/bin/env ruby | |||
There was a problem hiding this comment.
I found it a lot easier to have this TCP server be inside the rspec process, so there isn't a process boundary to navigate. I think it would have been necessary for it to be a docker image before to get around weird networking sadness.
| @@ -180,8 +180,6 @@ | |||
| end # }}} | |||
|
|
|||
| context 'receiving all unprompted input registers' do # {{{ | |||
| before { sleep 0.2 } # avoid test seeing a row write from a previous test | |||
There was a problem hiding this comment.
Can no longer happen \o/
|
|
||
| # Set up an lxp-bridge instance | ||
| config.before(:suite) do | ||
| Kernel.system(*%w[cargo build --manifest-path ../Cargo.toml]) |
There was a problem hiding this comment.
Ideally we'd run the integration test after the build step and have it use the produced binary, rather than needing all of rust to run the ruby integration suite.
There was a problem hiding this comment.
I am tempted to drop the build steps from CI that build native binaries, at least for PRs, and instead build a Docker image. Then we can use that for the integration tests?
The native builds are a bit of a relic from pre-docker days and I doubt anyone uses them to be honest. Could keep the binaries builds for releases as thats completely separate CI anyway but even then I think we should encourage Docker-only use.
There was a problem hiding this comment.
In this PR, I'm starting a new lxp-bridge instance per test. I was starting a new mosquitto per test, using docker, but that's weirdly broken inside CI, so I've stopped: #149 (comment) . I expect an lxp-bridge docker container would have the same issue.
A docker image would just be the same binary sat inside a squashfs archive, with a few other gubbins; I wouldn't use docker for running lxp-bridge myself (I run on quite constrained hardware; it doesn't even have docker installed), but I don't much mind how CI works - as long as it does work 😅.
|
It's green \o/ |
| config.before(:suite) do | ||
| RSpec.configuration.mqtt = MQTT::Client.connect('mqtt://localhost') | ||
| end | ||
| # Set up an MQTT server |
There was a problem hiding this comment.
These blocks should really go into files in spec/support
lupine
force-pushed
the
nt-isolated-integration-tests
branch
from
0b9bb24 to
7876fea
Compare
celsworth
force-pushed
the
master
branch
2 times, most recently
from
d41335a to
6043637
Compare
Don't mind me, just trying to get the integration tests working according to #147 (comment)
With the isolated approach, they're currently taking ~45 seconds to run locally, factoring out the cargo build part.