Integration of Linter with Jest Presets

[reficul31]

Refs Issue 1 and Issue 2. The PR consists of the integration of the Linter with the jest presets and also the unit tests for the activity logger module. This PR also sets up the basic testing suite with jest.
Not sure whether the linter should watch the test files as well as the src files, therefore I have left that part blank.

[Treora]

Nice. Jest looks good to me, had not heard of it before. To simplify discussion, it would be easier to put the tests themselves in a separate PR. I'll just comment on the jest setup itself now.

[reficul31]

To simplify discussion, it would be easier to put the tests themselves in a separate PR.

I thought a couple of testing suites will help visualize how the whole setup will turn out. I also needed a few tests to check if the environment was functioning the way it was supposed to. Although if it is a causing ambiguity as to the purpose of the PR I will be sure to break it up into two PRs.

[reficul31]

I think we can do without the mock for pouchdb, it would be bothersome to have to keep the two files in sync. Not sure what's the cleanest way. Adding a test for env='test' in src/pouchdb.js is one option.

@sanjo

Another thing, do you have a preference for having all tests in one folder, as you have done now, or would you think putting .test.js files besides each file is a nicer approach? I have often seen the latter, it makes a larger mess in folders but avoids having to create and maintain the same folder structure twice, and makes it easier for developers to quickly open the test for a file.

If it were totally up to me, I would use the different folder approach as I feel it keep the code quite clean and segregated. But hey, that's just me. I feel that we should ask my mentors for their advice on this topic. @sanjo @obsidianart @danrl @raj-maurya

[reficul31]

Changes made

• Removed globals from .eslintrc
• Added the comment to suppress the error
• Rearranged the commits to make more sense

[ghost]

I think we can do without the mock for pouchdb, it would be bothersome to have to keep the two files in sync. Not sure what's the cleanest way. Adding a test for env='test' in src/pouchdb.js is one option.

Let's do this. Jest will set NODE_ENV automatically to 'test' (Reference).

Another thing, do you have a preference for having all tests in one folder, as you have done now, or would you think putting .test.js files besides each file is a nicer approach? I have often seen the latter, it makes a larger mess in folders but avoids having to create and maintain the same folder structure twice, and makes it easier for developers to quickly open the test for a file.

I prefer to have unit test files next to the implementation files because it makes it significantly easier to open them when you work on a module.

[reficul31]

I have moved the test files to the src folders, however I can't seem to figure out how to check the environment of node and change the configuration of the adapter. Currently I am trying to do the following.

let db = new PouchDB({
    name: 'webmemex',
    auto_compaction: true,
})

if (process.env.NODE_ENV === 'test') {
    db = new PouchDB({
        name: 'testdb',
        auto_compaction: true,
        adapter: 'memory',
    })
}

I know the logic is extremely flawed and I would really appreciate some help.

[ghost]

The check should be correct. Do you get any error? You can also probably use console.log to check, if it works correctly.

let db
if (process.env.NODE_ENV === 'test') {
    db = new PouchDB({
        name: 'testdb',
        auto_compaction: true,
        adapter: 'memory',
    })
} else {
    db = new PouchDB({
        name: 'webmemex',
        auto_compaction: true,
    })
}

[reficul31]

Using let db throws a linting error stating Exporting mutable 'let' binding, use 'const' instead . Currently I have suppressed the error, but I would really like a better way to do this.

[Treora]

Using let db throws a linting error stating Exporting mutable 'let' binding, use 'const' instead .

Can always work around that if needed:

let db_
...
const db = db_

Or maybe export {db_ as db} would also work?

Or one can do something like:

const db = (process.env.NODE_ENV === 'test')
    ? ....
    : ....

By the way, it would be nice if the extension can keep importing only pouchdb-browser, not whole pouchdb (=pouchdb-browser+pouchdb-node), to reduce bundle size. The test version could probably just use pouchdb-node, actually, and require it to import it conditionally.

Maybe something like this?:

import PouchDB from 'pouchdb-browser'

const pouchdbOptions = {
    name: 'webmemex',
    autocomplete: true,
}

function initDB() {
    return PouchDB(pouchdbOptions)
}

function initTestDB() {
    const PouchDB = require('pouchdb-node').default
    return PouchDB({
        ...options,
        name: 'testdb',
        adapter: 'memory',
    })
}

export const db = (process.env.NODE_ENV === 'test')
    ? initTestDB()
    : initDB()

[reficul31]

Changes made

• Implementation of the db configuration based upon @Treora suggestion
• Removed some dependencies that weren't required

[ghost]

We might also update the contribution guidelines to require or at least recommend that developers write unit tests for the code they add or modify.

[reficul31]

@Treora I think the PR is ready to merge :D

[reficul31]

So after a lot of thought, I thought of going with second suggestion given by @Sanjo . Don't really have a reason, but mainly I was actively trying to avoid another dependency, in this case if I went with two configs that dependency would have been gulp-ignore.

[reficul31]

@Treora any more thoughts on the PR?

[reficul31]

Changes made:

• Removed all the un-necessary rules for the testing suite.
• Added a jest config file that is being passed as --config
• Changed the config of testdb

[blackforestboi]

Yeah! congrats @reficul31 :)

I cannot tell if its a good job, but I assume, since you passed Gerben's requirements :P