ssbc / ssb-config

standard configuration for ssb
MIT License
23 stars 18 forks source link

add tests to check servers start and can be connected to #33

Closed mixmix closed 5 years ago

mixmix commented 5 years ago

WHY - when I started writing docs for ssb-config, the default config generated did not let you run a working ssb-server as advertised

I think it's imperative that people can get started and try out scuttlebutt, and having ssb-config in a state where you can't start sbot is a great way to put people off.

I've added a script npm run test:lite for those who don't want to run the server tests because they are currently running an sbot

mixmix commented 5 years ago

This is currently broken because I can't get a server started and a client connected to it. How fucking embarassing D: I've been working on this for more than an hour and don't know what's wrong.

The defaultConfig is generating a host of ::, which I guess is ipv6... but this isn't causing trouble for e.g. patchbay startup and connection (even when I force it to connect over net)

Here's the output:

Error: could not connect to sbot
    at /home/mix/projects/SSBC/ssb-client/index.js:102:23
    at /home/mix/projects/SSBC/ssb-client/node_modules/multiserver/compose.js:66:24
    at Socket.<anonymous> (/home/mix/projects/SSBC/ssb-client/node_modules/multiserver/plugins/net.js:42:11)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)
    at emitErrorNT (internal/streams/destroy.js:64:8)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9)
  Error: connect ECONNREFUSED :::8008
    at Object._errnoException (util.js:992:11)
    at _exceptionWithHostPort (util.js:1014:20)
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1186:14)
not ok 23 remote connection to server works
  ---
    operator: notOk
    expected: |-
      false
    actual: |-
      [Error: could not connect to sbot]
    at: Client (/home/mix/projects/SSBC/ssb-config/tests/server-startup.js:37:14)
    stack: |-
      Error: could not connect to sbot
          at /home/mix/projects/SSBC/ssb-client/index.js:102:23
          at /home/mix/projects/SSBC/ssb-client/node_modules/multiserver/compose.js:66:24
          at Socket.<anonymous> (/home/mix/projects/SSBC/ssb-client/node_modules/multiserver/plugins/net.js:42:11)
          at emitOne (events.js:116:13)
          at Socket.emit (events.js:211:7)
          at emitErrorNT (internal/streams/destroy.js:64:8)
          at _combinedTickCallback (internal/process/next_tick.js:138:11)
          at process._tickCallback (internal/process/next_tick.js:180:9)
        Error: connect ECONNREFUSED :::8008
          at Object._errnoException (util.js:992:11)
          at _exceptionWithHostPort (util.js:1014:20)
          at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1186:14)
  ...
mixmix commented 5 years ago

cc @arj03 / @christianbundy if you have any spare cycles and can spot anything stupid I'm doing in these tests would love to know

mixmix commented 5 years ago

oh I think I got a solution ... need to use a child_process

mixmix commented 5 years ago

This is ready to go

dominictarr commented 5 years ago

utils/test-server.js seems to be empty? also please put test utils under test directory.

I'd also rather not have tests that start a server on the default port, because it will break if you are running ssb-server... okay at somepoint maybe there can be an integration test for this... but it could risk being really annoying

mixmix commented 5 years ago

Yeah I get you @dominictarr , but the default setup being known to be working is worth the hassle - the alternative of having a bung default config and a persons first experience trying to build with scuttlebutt being shit is just too dangerous.

Note that I added a script for you to run the tests without starting servers. Is this an acceptable comprimise?

dominictarr commented 5 years ago

reflecting on this more... yes we need stuff like this, but this is an integration test

It's very different to a unit test and shouldn't be mixed in with unit tests... given the ssb-server@14 refactor, ssb-server now just being a distribution of plugins, with a shrinkwrap - I think this is the correct place to be running integration tests. Also, since integration tests are usually slow and annoying, that's another good reason to separate them.

I'm very apprehensive about running tests that use load the local user's data - those could do all sorts of bad things, one very plausible annoying thing is rebuild the indexes. (not the end of the world, but enough to make you curse running the tests). loading the user data's shouldn't be the default.

dominictarr commented 5 years ago

though, on that note... we don't actually have any organized integration tests. there is ssb-server/test/bin.js. That needs to be refactored out anyway, since I want other distributions to run it before they publish.

dominictarr commented 5 years ago

so we need to put it somewhere... so my ask @mixmix is it okay if we make a place for integration tests to go and put this test there?

mixmix commented 5 years ago

Yeah I'm comfortable with that as long as there's a clear pointer.

The other major problem I've been thinking about it ssb-config is really a sub-module of ssb-server - there should be some link (even if only in readme) about which version of ssb-server this config generating works for. Or is this really 100% backwards compatible forever.

Mmm. At the moment I'd rather merge this then have someone extract integration tests. Tbh these tests here take no time at all to run, so I'm not quite sure what your fear is Maybe the reason these don't feel unit like is because ssb-config is not a unit in and of itself.

On Sun, 20 Jan 2019, 23:49 Dominic Tarr, notifications@github.com wrote:

so we need to put it somewhere... so my ask @mixmix https://github.com/mixmix is it okay if we make a place for integration tests to go and put this test there?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ssbc/ssb-config/pull/33#issuecomment-455855650, or mute the thread https://github.com/notifications/unsubscribe-auth/ACitnv7FqHxA5zVvnQ33r4UXmE7I83i8ks5vFEnFgaJpZM4aJCTH .

mixmix commented 5 years ago

I've merged this so it doesn't go stale.

I'm quite happy for these tests to be refactored somewhere later in a follow up PR once we figure out what integration tests look like and a good home for them