rrdelaney/ava-rethinkdb

:link: RethinkDB helpers for AVA

ava-rethinkdb's Issues

process.cwd() conflicts in cleanup

cleanup() nukes all database storage directories in process.cwd(), which would likely cause conflicts between files.

In newer versions of AVA, process.cwd() returns the directory containing package.json, not the directory containing the test file (avajs/ava#1074), which makes the problem even worse, as all test files will conflict with each other.

I'm planning to make a series of changes to update ava-rethinkdb, including fixing this issue.

• ava-21

  • Update ava in devDependency to use 0.21.0, the latest version
  • Add ava to peerDependencies with versions 0.21.x and < 0.21.0, so users can't use newer versions of AVA with breaking changes.
  • Add a .travis.yml file for Travis CI testing, testing against Node.js 6 and 8 and AVA v0.15.2 and v0.21.0

• test-instances

  • Add AVA tests that use ava-rethinkdb from multiple instances, to simulate the final example in the README

• fix-cwd

  • Update init() and cleanup() to be less dependent on process.cwd()
    Potentially use ${process.cwd()}/${process.pid}/${port}, and then nuke ${process.pid} in cleanup()

Let me know if these changes are appealing, and I'll work on some PRs! I'm currently just drafting changes in my fork, so I'll probably need to take a second pass to make sure that I conform to your style.

Ability to set database password

This is a feature request to support passing options to RethinkDB from the init() method.

In particular, I want to set RethinkDB's --initial-password option. I imagine doing it like this...

import test from 'ava';
import { init, cleanup } from 'ava-rethinkdb';

const initialData = {};
const option = {
    password : 'abcdefg'
};

test.before(init(initialData, option));
test.after.always(cleanup);

Now, you might ask, why would I want to set a password, since it's just a test?

Well, it would make it much easier to test my app directly, because my app enforces the use of a database password in order to avoid mistakes that could have nasty consequences in production. I don't want to conditionally enforce it based on environment configuration, as that is something very prone to error as well. My preferred solution is to just always have a password. In the tests, the password can be hardcoded and that's fine.

Great project, btw! I hope it will help more people adopt RethinkDB. Now if only I could install the database from npm...

Reorganize Plugin, Implement Timeouts

ava-rethinkdb is currently hanging. It's consistently hanging on my local machine (Node 8), and is quite flaky in CI (jobs needs to be restarted often).

I'd like to take a slightly drastic approach to fix this.

• Move functionality to a separate class (MockedRethinkDB.js)

I'd like to move the actual functionality of the mocking into a separate file.
This will split testing, and let us actually test the functionality of each component, rather than just integration tests which are harder to debug.

• Split functionality into separate methods

The plugin attempts to do many things all in the init() stage:

• Select a random port
• Create temporary directory for storage
• Set DEFAULT_PORT inside require("rethinkdb")
• Spawn RethinkDB, wait for start
• Import data into RethinkDB

As some of these appear to be more stable than others, it would be nice to break each down into a separate method so each stage can be unit-tested separately. I'd also like to add (configurable) timeouts tweaked for each stage, so CI pipelines won't hang with no additional information.

• Add require("ava-rethinkdb").setup()

init() is made to be inserted into a single beforeEach().
With the functionality split into separate methods, it could be nice to have a beforeEach() statement for each stage to provide more context for debugging (especially if stages timed-out instead of just hanging).
setup() could even contain the afterEach for cleanup().

setup() would be called in the top-level scope:

const test = require("ava");
const { setup } = require("ava-rethinkdb");

setup();

test(...);

Then setup() would create each of the beforeEach() and afterEach() statements needed, and be able to give each a descriptive name to narrow down debugging scope.

I'd retain init() for backwards-compatibility, and still have it concatenate each method from MockedRethinkDB.js into a single beforeEach() stage, but strongly suggest setup() in the README.

Final thought: it might be better if setup() had a name relating to ava-rethinkdb so we could use the object unpacking without conflicting with other modules that might want to use setup() as a keyword.
Instead, maybe it should be named mockRethink() or something.

Planned Breaking Changes

I'm currently drafting my work with a few breaking changes for now. I can tweak the code for many of these if they are troublesome.

• init() takes more options than just initialData, so initialData is a layer deeper in the configuration object.
• init() and cleanup() need to be used in beforeEach() and afterEach() instead of before() and after(). This allows using t.context to store internal information, including the port used, allowing retrying ports.

(@rrdelaney)

Code Coverage

I'm not the biggest fan of using code coverage to drive development, but I sometimes reference the report to make sure that tests hit what is expected.

Codecov is simple to setup, while Code Climate also deals with code linting.

@rrdelaney would you like one of these configured for ava-rethinkdb?

Error: spawn rethinkdb ENOENT

When I run the ava-rethinkdb it works but when I ran it through travis CI
I get Error: spawn rethinkdb ENOENT

Is there something I am doing wrong or need to add for ci build?

Start CHANGELOG

@rrdelaney I've got a CHANGELOG template that I've been using: https://github.com/rweda/configurable-arbitrary/blob/master/CHANGELOG.md

Any problem with using that format for a CHANGELOG?

Add Travis badge to README