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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to