You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2021/06/23 16:22:12 UTC
[cordova-android] branch master updated: refactor(env/java):
improve tests and implementation (#1246)
This is an automated email from the ASF dual-hosted git repository.
raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-android.git
The following commit(s) were added to refs/heads/master by this push:
new 0f13f4a refactor(env/java): improve tests and implementation (#1246)
0f13f4a is described below
commit 0f13f4a5ac8e657aee9ee52f1aabf1c59ccdf3b9
Author: Raphael von der Grün <ra...@gmail.com>
AuthorDate: Wed Jun 23 18:22:04 2021 +0200
refactor(env/java): improve tests and implementation (#1246)
This basically fixes up the changes from #1220.
* test(env/java): replace test that duplicates implementation
* test(env/java): stub _ensure to focus on unit under test
* test(env/java): add test for invalid output
* refactor(env/java): keep try block small
* refactor(env/java): shorten excessive comment
---
bin/templates/cordova/lib/env/java.js | 31 +++++++------------------------
spec/unit/check_reqs.spec.js | 19 -------------------
spec/unit/java.spec.js | 33 ++++++++++++++++++++-------------
3 files changed, 27 insertions(+), 56 deletions(-)
diff --git a/bin/templates/cordova/lib/env/java.js b/bin/templates/cordova/lib/env/java.js
index 5343beb..2a0c39d 100644
--- a/bin/templates/cordova/lib/env/java.js
+++ b/bin/templates/cordova/lib/env/java.js
@@ -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);
@@ -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]);
},
/**
diff --git a/spec/unit/check_reqs.spec.js b/spec/unit/check_reqs.spec.js
index 68c3d38..6547200 100644
--- a/spec/unit/check_reqs.spec.js
+++ b/spec/unit/check_reqs.spec.js
@@ -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 () {
diff --git a/spec/unit/java.spec.js b/spec/unit/java.spec.js
index 66c8ef2..f57e9fc 100644
--- a/spec/unit/java.spec.js
+++ b/spec/unit/java.spec.js
@@ -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 () => {
@@ -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'
}));
@@ -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', () => {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org