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

cli: add simple integration tests #937

Closed
wants to merge 5 commits into from
Closed

cli: add simple integration tests #937

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3432f3d
cli: add simple integration tests
Jul 27, 2023
ce8fa27
expand tests, improve output on fail
Sep 6, 2023
d30b9d7
try test-cntainer
Sep 6, 2023
02fa0d9
wip
Sep 6, 2023
1701bd3
revert testContainer()
Sep 6, 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
53 changes: 53 additions & 0 deletions test/integration/bin/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
const { execSync } = require('node:child_process');
const uuid = require('uuid').v4;

describe('cli', () => {
it('should return status code 1 if no command is issued', () => {
let thrown = false; // pattern from test/unit/util/crypto.js
try {
// eslint-disable-next-line no-use-before-define
cli('', { stdio: ['pipe', 'pipe', 'ignore'] });
} catch (err) {
err.status.should.equal(1);
thrown = true;
}
thrown.should.equal(true);
});

it('should return status code 1 if non-existent command is issued', () => {
let thrown = false; // pattern from test/unit/util/crypto.js
try {
// eslint-disable-next-line no-use-before-define
cli('user-congratulate', { stdio: ['pipe', 'pipe', 'ignore'] });
} catch (err) {
err.status.should.equal(1);
thrown = true;
}
thrown.should.equal(true);
});

describe('command: user-create', () => {
it('should return status code 0 and user details on success', () => {
Copy link
Member

Choose a reason for hiding this comment

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

It feels unclear to me what should be tested in this file vs. test/integration/task/account.js. It makes sense that this file is testing status codes. For testing output, my instinct is that that should be done in test/integration/task/account.js along with the other tests related to the behavior of each task.

Copy link
Contributor Author

@alxndrsn alxndrsn Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is an integration test for lib/bin/cli.js. I'd be surprised to find it in test/integration/task/account.js. Perhaps cli.js is named misleading though, as it only deals with accounts(?)

Copy link
Member

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

Choose a reason for hiding this comment

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

I think that's right that cli.js only deals with accounts. It may have been named the way it was with the thought that it would eventually do more things. In the user docs, we mention an odk-cmd, which just calls cli.js I think. As far as I know, the only other script that users run directly is restore.js for restoring a backup (user docs).

// eslint-disable-next-line no-use-before-define
const email = randomEmail();

// eslint-disable-next-line no-use-before-define
const [, , js] = cli(`--email ${email} user-create`, { input: 'strong-password-101' })
.match(/^([^{]*)(.*)$/ms);

js.should.match(new RegExp(`email: '${email}',`));
js.should.not.match(/password/);
});
});
});

function cli(argString, opts) {
return execSync(
'node lib/bin/cli.js ' + argString,
{ encoding: 'utf-8', ...opts },
);
}

function randomEmail() {
return uuid() + '@example.com';
}