Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
"ArrayBuffer": "readonly",
"FileReaderSync": "readonly",
"emit": "readonly",
"PouchDB": "readonly"
"PouchDB": "readonly",
"__pouch__": "readonly"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could window.__pouch__ could be used instead of adding an extra constant here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omitting window. from the test code was a readability win to me but I don't have a strong preference either way. Happy to change this if it's a big issue.

},

"rules": {
Expand Down
24 changes: 24 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,30 @@ jobs:
if: steps.retry.outcome == 'failure'
run: ${{ matrix.cmd }}

# Run the multitab tests in every supported browser

multitab:
needs: lint
strategy:
fail-fast: false
matrix:
client: ['firefox', 'chromium', 'webkit']
adapter: ['idb', 'indexeddb']
runs-on: ubuntu-latest
env:
CLIENT: ${{ matrix.client }}
ADAPTERS: ${{ matrix.adapter }}
steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
- uses: ./.github/actions/install-node-package
with:
node-version: ${{ env.NODE_VERSION }}
- uses: ./.github/actions/build-pouchdb
- run: npx playwright install --with-deps ${{ matrix.client }}
- run: npm run test-multitab

# Run the integration, find and mapreduce tests against all the Node.js
# PouchDB adapters. This should be run for every adapter on every version of
# Node.js we support.
Expand Down
19 changes: 19 additions & 0 deletions TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ help fixing them though.
- [Test options](#test-options)
- [Other sets of tests](#other-sets-of-tests)
- [`find` and `mapreduce`](#find-and-mapreduce)
- [multi-tab tests](#multi-tab-tests)
- ["Fuzzy" tests](#fuzzy-tests)
- [Performance tests](#performance-tests)
- [Running tests in the browser](#running-tests-in-the-browser)
Expand Down Expand Up @@ -235,6 +236,24 @@ like so:
Note that the default choice for the `SERVER` value (`pouchdb-express-router`)
does not support `find` or `mapreduce` and does not need to pass these tests.

### Multi-tab tests

We have a few tests to check that interactions with backing stores like
IndexedDB are synchronized across tabs where multiple PouchDB instances are
connected to the same database. These have to be scripted via Playwright rather
than being run inside the normal unit test suite. To run them:

$ npm run test-multitab

Two environment variables can be used to control how these are run:

- `CLIENT`: may be `firefox`, `chromium` or `webkit` to select the browser where
the tests will be run.
- `ADAPTERS`: may be `idb` or `indexeddb` to set which backend adapter PouchDB
will use.

All combinations of these should be tested on CI.

### "Fuzzy" tests

This test suite checks some more unusual replication scenarios, it can be run
Expand Down
14 changes: 14 additions & 0 deletions bin/test-multitab.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';

const { spawn } = require('child_process');
const devserver = require('./dev-server');

const MOCHA_BIN = './node_modules/.bin/mocha';
const MULTITAB_TESTS = 'tests/multitab';
const TIMEOUT = '10000';

devserver.start(() => {
let argv = ['-t', TIMEOUT, MULTITAB_TESTS];
let mocha = spawn(MOCHA_BIN, argv, { stdio: 'inherit' });
mocha.on('close', (status) => process.exit(status));
});
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"test-component": "mocha --exit tests/component # --exit for #8839",
"test-fuzzy": "TYPE=fuzzy npm run test",
"test-browser": "node ./bin/test-browser.js",
"test-multitab": "node ./bin/test-multitab.js",
"test-memleak": "mocha -gc tests/memleak",
"eslint": "eslint --cache bin/ packages/node_modules/**/src tests/",
"dev": "./bin/run-dev.sh",
Expand Down
14 changes: 9 additions & 5 deletions packages/node_modules/pouchdb-adapter-indexeddb/src/bulkDocs.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ import { DOC_STORE, META_LOCAL_STORE, idbError } from './util';
import { rewrite, sanitise } from './rewrite';
const sanitisedAttachmentKey = sanitise('_attachments');

export default function (api, req, opts, metadata, dbOpts, idbChanges, callback) {
export default function (api, req, opts, cachedMeta, dbOpts, idbChanges, callback) {

let txn;
let metadata = {};

// TODO: I would prefer to get rid of these globals
let error;
Expand Down Expand Up @@ -320,12 +321,12 @@ export default function (api, req, opts, metadata, dbOpts, idbChanges, callback)
} catch (e) {
return Promise.reject(createError(BAD_ARG, 'Attachment is not a valid base64 string'));
}
if (metadata.idb_attachment_format === 'binary') {
if (cachedMeta.idb_attachment_format === 'binary') {
attachment.data = binStringToBlobOrBuffer(binData, attachment.content_type);
}
} else {
binData = attachment.data;
if (metadata.idb_attachment_format === 'base64') {
if (cachedMeta.idb_attachment_format === 'base64') {
// TODO could run these in parallel, if we cared
return new Promise(resolve => {
blufferToBase64(attachment.data, function (b64) {
Expand Down Expand Up @@ -412,8 +413,11 @@ export default function (api, req, opts, metadata, dbOpts, idbChanges, callback)
callback(null, results);
};

// We would like to use promises here, but idb sucks
fetchExistingDocs(txn, docs);
const metaStore = txn.objectStore(META_LOCAL_STORE);
metaStore.get(META_LOCAL_STORE).onsuccess = function (e) {
Object.assign(metadata, e.target.result);
fetchExistingDocs(txn, docs);
};
});
}).catch(function (err) {
callback(err);
Expand Down
42 changes: 28 additions & 14 deletions packages/node_modules/pouchdb-adapter-indexeddb/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ function IndexeddbPouch(dbOpts, callback) {
}

const api = this;
let metadata = {};
let cachedMeta = {};

// Wrapper that gives you an active DB handle. You probably want $t.
const $ = function (fun) {
return function () {
const args = Array.prototype.slice.call(arguments);
setup(openDatabases, api, dbOpts).then(function (res) {
metadata = res.metadata;
cachedMeta = res.metadata;
args.unshift(res.idb);
fun.apply(api, args);
}).catch(function (err) {
Expand All @@ -60,9 +60,8 @@ function IndexeddbPouch(dbOpts, callback) {
const args = Array.prototype.slice.call(arguments);

return setup(openDatabases, api, dbOpts).then(function (res) {
metadata = res.metadata;
cachedMeta = res.metadata;
args.unshift(res.idb);

return fun.apply(api, args);
});
};
Expand All @@ -78,7 +77,7 @@ function IndexeddbPouch(dbOpts, callback) {
const args = Array.prototype.slice.call(arguments);
const txn = {};
setup(openDatabases, api, dbOpts).then(function (res) {
metadata = res.metadata;
cachedMeta = res.metadata;
txn.txn = res.idb.transaction(stores, mode);
}).catch(function (err) {
console.error('Failed to establish transaction safely');
Expand All @@ -97,29 +96,44 @@ function IndexeddbPouch(dbOpts, callback) {
}, stores, mode)(callback);
};

api._getMetadata = $t(function (txn, cb) {
const metaStore = txn.txn.objectStore(META_LOCAL_STORE);
metaStore.get(META_LOCAL_STORE).onsuccess = function (e) {
cb(null, e.target.result);
};
}, [META_LOCAL_STORE]);

api._remote = false;
api.type = function () { return ADAPTER_NAME; };

api._id = $(function (_, cb) {
cb(null, metadata.db_uuid);
});
api._id = function (cb) {
api._getMetadata(function (err, metadata) {
cb(null, metadata.db_uuid);
});
};

api._info = $(function (_, cb) {
return info(metadata, cb);
});
api._info = function (cb) {
api._getMetadata(function (err, metadata) {
return info(metadata, cb);
});
};

api._get = $t(get, [DOC_STORE]);
api._getLocal = $t(function (txn, id, callback) {
return getLocal(txn, id, api, callback);
}, [META_LOCAL_STORE]);

api._bulkDocs = $(function (_, req, opts, callback) {
bulkDocs(api, req, opts, metadata, dbOpts, idbChanges, callback);
bulkDocs(api, req, opts, cachedMeta, dbOpts, idbChanges, callback);
});

api._allDocs = $t(function (txn, opts, cb) {
allDocs(txn, metadata, opts, cb);
}, [DOC_STORE]);
const metaStore = txn.txn.objectStore(META_LOCAL_STORE);
metaStore.get(META_LOCAL_STORE).onsuccess = function (e) {
const metadata = e.target.result;
allDocs(txn, metadata, opts, cb);
};
}, [DOC_STORE, META_LOCAL_STORE]);

api._getAttachment = getAttachment;

Expand Down
42 changes: 42 additions & 0 deletions tests/multitab/concurrency.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
'use strict';

const { assert } = require('chai');
const UserAgent = require('./user_agent');

describe('multi-tab concurrency', () => {
let agent, res;

beforeEach(async () => {
agent = await UserAgent.start();
});

afterEach(async () => {
await agent.stop();
});

async function checkInfo({ doc_count }) {
let info1 = await agent.eval(1, () => __pouch__.info());
assert.equal(info1.doc_count, doc_count);

let info2 = await agent.eval(2, () => __pouch__.info());
assert.deepEqual(info1, info2);
}

it('creates docs concurrently in two tabs', async () => {
res = await agent.eval(1, () => __pouch__.put({ _id: 'doc-1' }));
assert(res.ok);
await checkInfo({ doc_count: 1 });

res = await agent.eval(2, () => __pouch__.put({ _id: 'doc-2' }));
assert(res.ok);
await checkInfo({ doc_count: 2 });

res = await agent.eval(1, () => __pouch__.put({ _id: 'doc-3' }));
assert(res.ok);
await checkInfo({ doc_count: 3 }); // fails on indexeddb; pages have different info

res = await agent.eval(2, () => __pouch__.put({ _id: 'doc-4' }));
assert(res.ok); // fails on indexeddb
await checkInfo({ doc_count: 4 });
});
});
9 changes: 9 additions & 0 deletions tests/multitab/shell.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!doctype html>
<html>
<head>
<title>Playwright shell</title>
<script src="/packages/node_modules/pouchdb/dist/pouchdb.min.js"></script>
<script src="/packages/node_modules/pouchdb/dist/pouchdb.indexeddb.min.js"></script>
</head>
<body></body>
</html>
53 changes: 53 additions & 0 deletions tests/multitab/user_agent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
'use strict';

const playwright = require('playwright');

const ADAPTERS = process.env.ADAPTERS || 'indexeddb';
const CLIENT = process.env.CLIENT || 'firefox';
const SHELL_URL = 'http://127.0.0.1:8000/tests/multitab/shell.html';

class UserAgent {
static async start() {
let browser = await playwright[CLIENT].launch();
let context = await browser.newContext();
return new UserAgent(ADAPTERS, browser, context);
}

constructor(adapter, browser, context) {
this._adapter = adapter;
this._browser = browser;
this._context = context;
this._pages = new Map();
}

async stop() {
await this._browser.close();
}

async eval(pageId, fn) {
let page = await this._getPage(pageId);
return page.evaluate(fn);
}

_getPage(id) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"page" means "tab"? Maybe helpful to use consistent language?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

page is the playwright API language and it is consistent with that. Of course we can change it, but not sure if worth it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was unsure about this. Everyone discussing these issues uses "tab" but Playwright uses "page". Strictly speaking, a "tab" is a particular UI for operating multiple pages, but it's not the only one. Colloquially I think "multi-tab" communicates "several pages operating concurrently" better than "multi-page" which to me sounds like a multi-step sequential workflow.

if (!this._pages.has(id)) {
this._pages.set(id, this._setupPage());
}
return this._pages.get(id);
}

async _setupPage() {
let page = await this._context.newPage();
await page.goto(SHELL_URL);

if (this._adapter === 'idb') {
await page.evaluate(() => window.__pouch__ = new PouchDB('testdb', { adapter: 'idb' }));
} else if (this._adapter === 'indexeddb') {
await page.evaluate(() => window.__pouch__ = new PouchDB('testdb', { adapter: 'indexeddb' }));
}

return page;
}
}

module.exports = UserAgent;
Loading