Run service tests on a given (remote) instance; test on [jetbrains] #2195
Conversation
platan
changed the title
There was a problem hiding this comment.
To be honest, it seems quite unnatural to me to run development-oriented test suites on remote/live instances. Once something is deployed and configured, in my opinion we should avoid tampering with it by running processes that do not pertain to its normal flow of execution. To check that an instance is working correctly after a deployment, wouldn't treating it as a black box by hitting it with a few badge requests be sufficient to assert that it is working properly?
Furthermore, our test suite is quite bulky and some services in particular have really slow tests. Others consume quite a few API calls from our rate limits, which is not great.
Those are my quick thoughts on the matter, I'll hand over to other maintainers for their opinions. 😉
|
I think this makes sense. It's one of the use cases I had in mind when I worked on #927. It could be a nicer way to run our nightly tests on staging.
Hehe, maybe not, though it's a good start, and it's basically what we do now. 🙃I might not wait for the entire suite to run, but hitting a few of the service tests in a reliable and automated way would make a nice post-deploy smoke tests. Running the full suite exercises a lot of code and provides a lot of confidence things are working. |
lib/test-config.js
Outdated
| // is followed by a colon | ||
| protocol: 'http:', | ||
| hostname: 'localhost', | ||
| port: 1111, |
There was a problem hiding this comment.
I see that this is trying to make things more DRY, though I wonder if it would be clearer to put the URL here, and also more consistent with the other URLs we configure. I also think this could be loaded from the environment, as it's a pretty unusual case.
Something like:
liveServerUrl: process.env.LIVE_SERVER_URL
services/service-tester.js
Outdated
| const specs = this.specs | ||
|
|
||
| const fn = this._only ? describe.only : describe | ||
| // eslint-disable-next-line mocha/prefer-arrow-callback | ||
| fn(this.title, function() { | ||
| specs.forEach(spec => { | ||
| spec.toss() | ||
| if (!options.skipIntercepted || !spec.intercepted) { |
There was a problem hiding this comment.
It's a bit difficult to follow how skipIntercepted is getting passed around here. I'm not sure I have a better solution, though.
There was a problem hiding this comment.
I used an environment variable to set this option via test-config and code looks better now in opinion. It's a separate commit and revert it if required 0974be4
lib/service-test-runner/runner.js
Outdated
| @@ -16,6 +20,9 @@ class Runner { | |||
| * Prepare the runner by loading up all the ServiceTester objects. | |||
| */ | |||
| prepare() { | |||
| if (this.options.testedServerUrl) { | |||
| testConfig.tested = this.options.testedServerUrl | |||
There was a problem hiding this comment.
It would be better not to mutate the test config; perhaps better to set it from the environment inside test-config, as I mention below.
There was a problem hiding this comment.
I agree with you, done :-)
chris48s
added
the
operations
|
Hey @platan, are you up for picking this up? |
|
@paulmelnikow I will be able to look at this next week. |
|
@paulmelnikow Thank you for the review. I've changed the code, diff is smaller and it should look better now. |
| module.exports = { | ||
| port: 1111, | ||
| get testedServerUrl() { |
There was a problem hiding this comment.
The getter is a nice way to handle runtime overriding!
This PR gives an option (
--url) to run service tests on a given instance. It can be used to check if given instance is working correctly, e.g. after deployment.Moreover I had to add an option (
--skip-intercepted) to skip tests which intercepts requests (byinterceptor bynetworkOff).An example command:
Is it useful? What do you think about it?