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

refactor(env/java): improve tests and implementation #1246

Merged
merged 5 commits into from
Jun 23, 2021
Merged

refactor(env/java): improve tests and implementation #1246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2d0ee3a
test(env/java): replace test that duplicates implementation
raphinesse Jun 23, 2021
833b0b8
test(env/java): stub _ensure to focus on unit under test
raphinesse Jun 23, 2021
d35d11b
test(env/java): add test for invalid output
raphinesse Jun 23, 2021
9f1f29f
refactor(env/java): keep try block small
raphinesse Jun 23, 2021
5effa2d
refactor(env/java): shorten excessive comment
raphinesse Jun 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 7 additions & 24 deletions bin/templates/cordova/lib/env/java.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,30 +41,9 @@ const java = {
await java._ensure(process.env);

// Java <= 8 writes version info to stderr, Java >= 9 to stdout
let version = null;
let javacOutput;
try {
const javacOutput = (await execa('javac', ['-version'], { all: true })).all;

/*
A regex match for the java version looks like the following:

version: [
'javac 1.8.0',
'1.8.0',
index: 45,
input: 'Picked up _JAVA_OPTIONS: -Xms1024M -Xmx2048M\njavac 1.8.0_271',
groups: undefined
]

We have to use a regular expression to get the java version, because on some environments
(e.g. macOS Big Sur) javac prints _JAVA_OPTIONS before printing the version and semver.coerce()
will fail to get the correct version from the output.
*/

const match = /javac\s+([\d.]+)/i.exec(javacOutput);
if (match && match[1]) {
version = match[1];
}
javacOutput = (await execa('javac', ['-version'], { all: true })).all;
} catch (ex) {
events.emit('verbose', ex.shortMessage);

Expand All @@ -79,7 +58,11 @@ const java = {
throw new CordovaError(msg);
}

return semver.coerce(version);
// We have to filter the output, because javac prints _JAVA_OPTIONS
// before printing the version which causes semver.coerce to fail to get
// the correct version if those options contain any numbers.
const match = /javac\s+([\d.]+)/i.exec(javacOutput);
return semver.coerce(match && match[1]);
},

/**
Expand Down
19 changes: 0 additions & 19 deletions spec/unit/check_reqs.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,6 @@ describe('check_reqs', function () {

await expectAsync(check_reqs.check_java()).toBeResolvedTo({ version: '1.8.0' });
});

it('should return the correct version if javac prints _JAVA_OPTIONS', async () => {
check_reqs.__set__({
java: {
getVersion: async () => {
let version = null;
const javacOutput = 'Picked up _JAVA_OPTIONS: -Xms1024M -Xmx2048M\njavac 1.8.0_271';
const match = /javac\s+([\d.]+)/i.exec(javacOutput);
if (match && match[1]) {
version = match[1];
}

return { version };
}
}
});

await expectAsync(check_reqs.check_java()).toBeResolvedTo({ version: '1.8.0' });
});
});

describe('check_android', function () {
Expand Down
33 changes: 20 additions & 13 deletions spec/unit/java.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,9 @@ describe('Java', () => {
const Java = rewire('../../bin/templates/cordova/lib/env/java');

describe('getVersion', () => {
let originalEnsureFunc = null;

beforeEach(() => {
/*
This is to avoid changing/polluting the
process environment variables
as a result of running these java tests; which could produce
unexpected side effects to other tests.
*/
originalEnsureFunc = Java._ensure;
spyOn(Java, '_ensure').and.callFake(() => {
return originalEnsureFunc({});
});
// No need to run _ensure, since we are stubbing execa
spyOn(Java, '_ensure').and.resolveTo();
});

it('runs', async () => {
Expand All @@ -66,7 +56,16 @@ describe('Java', () => {
expect(result.version).toBe('1.8.0');
});

it('produces a CordovaError on error', async () => {
it('detects JDK when additional details contain numbers', async () => {
Java.__set__('execa', () => Promise.resolve({
all: 'Picked up _JAVA_OPTIONS: -Xms1024M -Xmx2048M\njavac 1.8.0_271'
}));

const { version } = await Java.getVersion();
expect(version).toBe('1.8.0');
});

it('produces a CordovaError on subprocess error', async () => {
Java.__set__('execa', () => Promise.reject({
shortMessage: 'test error'
}));
Expand All @@ -79,6 +78,14 @@ describe('Java', () => {
.toBeRejectedWithError(CordovaError, /Failed to run "javac -version"/);
expect(emitSpy).toHaveBeenCalledWith('verbose', 'test error');
});

it('throws an error on unexpected output', async () => {
Java.__set__('execa', () => Promise.reject({
all: '-version not supported'
}));

await expectAsync(Java.getVersion()).toBeRejectedWithError();
});
});

describe('_ensure', () => {
Expand Down