[GitHub] [cordova-android] breautek commented on a change in pull request #1229: feat: CORDOVA_JAVA_HOME env variable

[GitBox <gi...@apache.org>]

breautek commented on a change in pull request #1229:
URL: https://github.com/apache/cordova-android/pull/1229#discussion_r628892610



##########
File path: spec/unit/java.spec.js
##########
@@ -76,6 +76,19 @@ describe('Java', () => {
             Java.__set__('javaIsEnsured', false);
         });
 
+        it('CORDOVA_JAVA_HOME overrides JAVA_HOME', async () => {
+            spyOn(utils, 'forgivingWhichSync').and.returnValue('');
+
+            const env = {
+                CORDOVA_JAVA_HOME: '/tmp/jdk'
+            };
+
+            await Java._ensure(env);
+
+            expect(env.PATH.split(path.delimiter))
+                .toContain(path.join(env.JAVA_HOME, 'bin'));

Review comment:
       Nope, I think I understand what you mean, this test could be improved. Let me know what you think about my change. I added an explicit assertion for `JAVA_HOME` as we want to ensure `JAVA_HOME` is set for our process (so that sub processes can be set appropriately), and changed it to be hard-coded `/tmp/jdk` as that is our chosen test value.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org