Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Test that Enketo IDs are requested during request to create form #1001

Merged
merged 2 commits into from
Oct 6, 2023
Merged

Test that Enketo IDs are requested during request to create form #1001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8df1533
Increase wait for Enketo response in tests
matthew-white Sep 22, 2023
a013b7a
Test that Enketo IDs are requested during request to create form
matthew-white Sep 22, 2023
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
12 changes: 6 additions & 6 deletions test/integration/api/forms/draft.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ describe('api: /projects/:id/forms (drafts)', () => {
should.not.exist(body.enketoId);
}));

it('should stop waiting for Enketo after 0.5 seconds @slow', testService(async (service) => {
it('should wait for Enketo only briefly @slow', testService(async (service) => {
const asAlice = await service.login('alice');
global.enketo.wait = (f) => { setTimeout(f, 501); };
global.enketo.wait = (done) => { setTimeout(done, 600); };
Copy link
Member Author

@matthew-white matthew-white Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The change from 501 to 600 is meant to address this intermittent test failure: #588 (comment). Note that this test was not introduced in #989, but rather #782. I don't think I did anything in #989 that would have changed the POST/GET …/draft endpoints. However, if this test is failing, it certainly seems possible that new tests in #989 could fail, so I think it's worth addressing now.

Here's one possible explanation for the failure:

  • A request to Enketo is sent. After 501 ms, the Enketo mock will respond.
  • A moment later, the endpoint time-bounds the request. After 500 ms has elapsed since that bounding, the endpoint will stop waiting for the request.
  • If "a moment later" is greater than 1 ms, then the test will fail.

Another possible explanation:

  • setTimeout() doesn't run callbacks after an exact number of milliseconds, but after at least the specified number of milliseconds. So the Enketo mock could respond after 502 ms instead of after 501 ms.
  • Similarly, the endpoint could stop waiting after 502 ms instead of after 500 ms.
  • In other words, both timers could exceed the number of milliseconds specified and maybe be processed together in a single batch of timers. In that case, the order in which the callbacks are run might be indeterminate.

I'm not sure what the exact explanation is, but the fact that the test is failing intermittently doesn't make me think that there's anything wrong with the functionality here. I think we just need to provide a little more wiggle room for the test.

await asAlice.post('/v1/projects/1/forms/simple/draft').expect(200);
const { body } = await asAlice.get('/v1/projects/1/forms/simple/draft')
.expect(200);
Expand Down Expand Up @@ -1181,13 +1181,13 @@ describe('api: /projects/:id/forms (drafts)', () => {
should.not.exist(form.enketoOnceId);
}));

it('should stop waiting for Enketo after 0.5 seconds @slow', testService(async (service) => {
it('should wait for Enketo only briefly @slow', testService(async (service) => {
const asAlice = await service.login('alice');
await asAlice.post('/v1/projects/1/forms')
.send(testData.forms.simple2)
.set('Content-Type', 'application/xml')
.expect(200);
global.enketo.wait = (f) => { setTimeout(f, 501); };
global.enketo.wait = (done) => { setTimeout(done, 600); };
await asAlice.post('/v1/projects/1/forms/simple2/draft/publish')
.expect(200);
const { body: form } = await asAlice.get('/v1/projects/1/forms/simple2')
Expand Down Expand Up @@ -1251,7 +1251,7 @@ describe('api: /projects/:id/forms (drafts)', () => {
it('should request Enketo IDs from worker if request from endpoint fails', testService(async (service, container) => {
const asAlice = await service.login('alice');

// First request to Enketo, from endpoint
// First request to Enketo, from the endpoint
await asAlice.post('/v1/projects/1/forms')
.send(testData.forms.simple2)
.set('Content-Type', 'application/xml')
Expand All @@ -1264,7 +1264,7 @@ describe('api: /projects/:id/forms (drafts)', () => {
should.not.exist(beforeWorker.enketoId);
should.not.exist(beforeWorker.enketoOnceId);

// Second request, from worker
// Second request, from the worker
global.enketo.callCount.should.equal(2);
await exhaust(container);
global.enketo.callCount.should.equal(3);
Expand Down
179 changes: 151 additions & 28 deletions test/integration/api/forms/forms.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const { DateTime } = require('luxon');
const { testService } = require('../../setup');
const testData = require('../../../data/xml');
const { exhaust } = require(appRoot + '/lib/worker/worker');
const { without } = require(appRoot + '/lib/util/util');

describe('api: /projects/:id/forms (create, read, update)', () => {

Expand Down Expand Up @@ -266,34 +267,6 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
body.draftToken.should.be.a.token();
})))));

it('should worker-process the form draft over to enketo', testService((service, container) =>
service.login('alice', (asAlice) =>
asAlice.post('/v1/projects/1/forms')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200)
.then(() => exhaust(container))
.then(() => asAlice.get('/v1/projects/1/forms/simple2/draft')
.expect(200)
.then(({ body }) => {
body.enketoId.should.equal('::abcdefgh');
should.not.exist(body.enketoOnceId);
})))));

it('should worker-process the published form over to enketo', testService((service, container) =>
service.login('alice', (asAlice) =>
asAlice.post('/v1/projects/1/forms?publish=true')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200)
.then(() => exhaust(container))
.then(() => asAlice.get('/v1/projects/1/forms/simple2')
.expect(200)
.then(({ body }) => {
body.enketoId.should.equal('::abcdefgh');
body.enketoOnceId.should.equal('::::abcdefgh');
})))));

it('should if flagged save the given definition as published', testService((service) =>
service.login('alice', (asAlice) =>
asAlice.post('/v1/projects/1/forms?publish=true')
Expand Down Expand Up @@ -324,6 +297,156 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
body.publishedAt.should.be.a.recentIsoDate();
})))));

describe('Enketo ID for draft', () => {
it('should request an enketoId', testService(async (service, { env }) => {
const asAlice = await service.login('alice');
const { body } = await asAlice.post('/v1/projects/1/forms')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
global.enketo.callCount.should.equal(1);
global.enketo.createData.should.eql({
openRosaUrl: `${env.domain}/v1/test/${body.draftToken}/projects/1/forms/simple2/draft`,
xmlFormId: 'simple2',
token: undefined
});
body.enketoId.should.equal('::abcdefgh');
should.not.exist(body.enketoOnceId);
}));

it('should return with success even if request to Enketo fails', testService(async (service) => {
const asAlice = await service.login('alice');
global.enketo.state = 'error';
const { body } = await asAlice.post('/v1/projects/1/forms')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
should.not.exist(body.enketoId);
should.not.exist(body.enketoOnceId);
}));

it('should wait for Enketo only briefly @slow', testService(async (service) => {
const asAlice = await service.login('alice');
global.enketo.wait = (done) => { setTimeout(done, 600); };
const { body } = await asAlice.post('/v1/projects/1/forms')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
should.not.exist(body.enketoId);
should.not.exist(body.enketoOnceId);
}));

it('should request an enketoId from worker if request from endpoint fails', testService(async (service, container) => {
const asAlice = await service.login('alice');

// First request to Enketo, from the endpoint
global.enketo.state = 'error';
await asAlice.post('/v1/projects/1/forms')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);

// Second request, from the worker
global.enketo.callCount.should.equal(1);
await exhaust(container);
global.enketo.callCount.should.equal(2);
const { body } = await asAlice.get('/v1/projects/1/forms/simple2')
.expect(200);
global.enketo.createData.should.eql({
openRosaUrl: `${container.env.domain}/v1/test/${body.draftToken}/projects/1/forms/simple2/draft`,
xmlFormId: 'simple2',
token: undefined
});
body.enketoId.should.equal('::abcdefgh');
should.not.exist(body.enketoOnceId);
}));

it('should not request an enketoId from worker if request from endpoint succeeds', testService(async (service, container) => {
const asAlice = await service.login('alice');
await asAlice.post('/v1/projects/1/forms')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
global.enketo.callCount.should.equal(1);
await exhaust(container);
global.enketo.callCount.should.equal(1);
}));
});

describe('Enketo IDs for published form', () => {
it('should request Enketo IDs', testService(async (service, { env }) => {
const asAlice = await service.login('alice');
const { body } = await asAlice.post('/v1/projects/1/forms?publish=true')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
global.enketo.callCount.should.equal(1);
without(['token'], global.enketo.createData).should.eql({
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we need to test the token argument at this time. Unlike in the draft case above, token won't be undefined. However, I'm not sure that the token is even needed for this flow to function: see #997. We don't test the token in existing tests.

openRosaUrl: `${env.domain}/v1/projects/1`,
xmlFormId: 'simple2'
});
body.enketoId.should.equal('::abcdefgh');
body.enketoOnceId.should.equal('::::abcdefgh');
}));

it('should return with success even if request to Enketo fails', testService(async (service) => {
const asAlice = await service.login('alice');
global.enketo.state = 'error';
const { body } = await asAlice.post('/v1/projects/1/forms?publish=true')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
should.not.exist(body.enketoId);
should.not.exist(body.enketoOnceId);
}));

it('should wait for Enketo only briefly @slow', testService(async (service) => {
const asAlice = await service.login('alice');
global.enketo.wait = (done) => { setTimeout(done, 600); };
const { body } = await asAlice.post('/v1/projects/1/forms?publish=true')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
should.not.exist(body.enketoId);
should.not.exist(body.enketoOnceId);
}));

it('should request Enketo IDs from worker if request from endpoint fails', testService(async (service, container) => {
const asAlice = await service.login('alice');

// First request to Enketo, from the endpoint
global.enketo.state = 'error';
await asAlice.post('/v1/projects/1/forms?publish=true')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);

// Second request, from the worker
global.enketo.callCount.should.equal(1);
await exhaust(container);
global.enketo.callCount.should.equal(2);
const { body } = await asAlice.get('/v1/projects/1/forms/simple2')
.expect(200);
without(['token'], global.enketo.createData).should.eql({
openRosaUrl: `${container.env.domain}/v1/projects/1`,
xmlFormId: 'simple2'
});
body.enketoId.should.equal('::abcdefgh');
body.enketoOnceId.should.equal('::::abcdefgh');
}));

it('should not request Enketo IDs from worker if request from endpoint succeeds', testService(async (service, container) => {
const asAlice = await service.login('alice');
await asAlice.post('/v1/projects/1/forms?publish=true')
.set('Content-Type', 'application/xml')
.send(testData.forms.simple2)
.expect(200);
global.enketo.callCount.should.equal(1);
await exhaust(container);
global.enketo.callCount.should.equal(1);
}));
});

it('should log the action in the audit log', testService((service) =>
service.login('alice', (asAlice) =>
asAlice.post('/v1/projects/1/forms')
Expand Down
5 changes: 4 additions & 1 deletion test/util/enketo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ const defaults = {
// The number of times that the mock has been called during the test, that is,
// the number of requests that would be sent to Enketo
callCount: 0,
// An object with a property for each argument passed to the create() method
createData: undefined,
Copy link
Member Author

Choose a reason for hiding this comment

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

I added createData instead of using receivedUrl in the new tests here, for two reasons:

  • This allows us to test the xmlFormId argument of enketo.create() (the second argument), not just the first argument (openRosaUrl/receivedUrl).
  • Consistency with editData elsewhere in this file, which we use to test enketo.edit().

// The OpenRosa URL that was passed to the create() method
receivedUrl: undefined,
// An object with a property for each argument passed to the edit() method
Expand Down Expand Up @@ -57,8 +59,9 @@ const request = () => {
});
};

const create = async (openRosaUrl) => {
const create = async (openRosaUrl, xmlFormId, token) => {
const { enketoId = '::abcdefgh' } = await request();
global.enketo.createData = { openRosaUrl, xmlFormId, token };
global.enketo.receivedUrl = openRosaUrl;
return { enketoId, enketoOnceId: '::::abcdefgh' };
};
Expand Down