dojo / cli-test-intern

:rocket: Dojo - cli command for testing applications
http://dojo.io
Other
2 stars 19 forks source link

JDK Dependency #51

Closed eheasley closed 6 years ago

eheasley commented 7 years ago

cli-test-intern currently defaults to using selenium tunnel for functional tests, which requires the JDK. We either need to deal with this better or at least note it as part of cli-test-intern in some way.

vansimke commented 7 years ago

This came up as an issue because I user didn't have the JDK installed and tried to run dojo test -b.... Since the browser tests use the Selenium tunnel, this appears to be a Dojo error. Lacking documentation, it could surprise users that they need the JDK when Dojo 2 normally only depends on other JS libraries.

kitsonk commented 7 years ago

There doesn't seem to be an easy way of detecting this when installing from npm. In theory, the CLI command could detect this, but also it is arguable that DigDug should be helping with this, and so have opened theintern/digdug#48 as well.

edhager commented 6 years ago

Below is a node script that could be used to detect if java is installed or not. We could add something like that as a post install script in package.json or the cli command could use it as a check each time the command runs.

@kitsonk thoughts?

const { exec } = require('child_process');

let found = false;
function noJava() {
    console.log('Java could not be detected.');
}

const javaVersion = exec('java -version', (err, stdout, stderr) => {
    if (err) {
        noJava();
    } else {
        let responseString = '';
        stdout && (responseString += stdout.toString());
        stderr && (responseString += stderr.toString());

        const pos = responseString.indexOf('java version');
        if (pos >= 0) {
            const version = responseString.slice(pos).split('\n')[0].split(' ')[2];
            console.log(`Detected Java version ${version}`);
            found = true;
        } else {
            noJava();
        }
    }
});

javaVersion.on('close', () => {
    process.exit(found ? 0 : 1);
});
kitsonk commented 6 years ago

That sounds like a good idea, of course modifying it to make sense in the usability of the command (which is essentially just warning people that they will not be able to run local tests without installing Java) I think @umaar ran into it too and got a bit frustrated and might have a comment about what would make good ergonomics on this.

umaar commented 6 years ago

Problem: Running dojo test -a presents me with this dialog:

image

Can things be rearchitected in such a way where if someone only cares about testing in Chrome (which I think is a common use case), a fast-track path is taken to launching puppeteer or something - so that no Java is needed?

For the warning: I like Ed's idea of running that script each time the user attempts to run functional tests and explaining why Java is needed, or linking to a webpage which explains it. I think it's important to run it as one of the first things so you don't have to wait for the app to be built, only to be told afterwards Java is required. Would also be good to get clarification on whether the JRE or JDK is needed.