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.

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

JDK Dependency #51

Closed
eheasley opened this issue Jul 19, 2017 · 5 comments
Closed

JDK Dependency #51

eheasley opened this issue Jul 19, 2017 · 5 comments

Comments

@eheasley
Copy link

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
Copy link
Contributor

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
Copy link
Member

kitsonk commented Jul 28, 2017

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
Copy link
Contributor

edhager commented Nov 6, 2017

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
Copy link
Member

kitsonk commented Nov 7, 2017

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
Copy link
Member

umaar commented Nov 7, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants