frontend tests: stale mocha.js version

[muxator]

File tests/frontend/lib/mocha.js is quite old: first committed in 2012 with ba4ebbb, slightly modified the day after in 7aee98b ("made test helpers more cross browser compatible").

-    var query = Mocha.utils.parseQuery(window.location.search || '');
-    if (query.grep) mocha.grep(query.grep);
+    //var query = Mocha.utils.parseQuery(window.location.search || '');
+    //if (query.grep) mocha.grep(query.grep);

It would be probably useful to update the frontend library like we did for the backend one.

[JohnMcLear]

This should be a case of visiting..

http://youretherpad:9001/tests/frontend/ and make sure it works.

Dropping in a new mocha.js

hitting up http://youretherpad:9001/tests/frontend/ and ensuring that the tests run as expected.

Some API changes may have happened in mocha since 2012 so they should be fixed as part of the pull request. See our contributing guidelines prior to submitting a PR :)

[JohnMcLear]

Relevant $100 GitPay sponsorship: https://gitpay.me/#/task/325

[Serdans]

I don't understand why it redirects to itself here. It gives me a ERR_TOO_MANY_REDIRECTS error when I access http://youretherpad:9001/tests/frontend. How do I go about running the tests?

etherpad-lite/src/node/hooks/express/tests.js

Lines 60 to 63 in 4ee5ddb

	args.app.get('/tests/frontend', function (req, res) {
	res.redirect('/tests/frontend/');
	});
	}

[JohnMcLear]

Just browsing to http://192.168.1.51:9001/tests/frontend/ works for me.. Try the trailing forward slash?

[Serdans]

Tried it with trailing forward slash also it gives me the same error. I'm running it on Windows does that have something to do with it?

[JohnMcLear]

Interesting, doesn't work for me on windows. Probably to do with the way it indexes test files to run.. We never test the backend on windows (it runs remotely on sauce labs and we mostly dev on linux/unix so..) I will create a separate issue for that.

[JohnMcLear]

Solution on #3804

TLDR; use /index.html ...

[alexanmtz]

@JohnMcLear, I'm trying to update Mocha to the most recent version (7.x) but the library was changed deeply and the new setup would modify a lot the way we run the tests today. With the new version we should use as a package, and I can see on package.json but the way we run on the browser today the project uses a standalone js.

So should I proceed with a version of mocha.js for with the standalone front-end available like this one?
https://github.com/mochajs/mocha/blob/2.2.5/mocha.js

[muxator]

As long as the change is done properly, and we do not lose the ability to to st, I see no blockers in updating to 7.0.

Do you have an idea of the steps involved?

[alexanmtz]

@muxator, yeah, I found the js on https://mochajs.org/#running-mocha-in-the-browser

And I need to make the proper changes on the runner, then see the tests running on http://0.0.0.0:9001/tests/frontend/index.html

[alexanmtz]

@muxator, I was able to update Mocha to the latest version, but there's a huge change in the way the reporters are extended. So this implementation on runner.js to extend the HTML reporter will no longer work:
https://github.com/ether/etherpad-lite/blob/develop/tests/frontend/runner.js#L69

And the Base function is included on the Mocha core

I'm trying different approaches but we can't require the module, to extend like this:
https://github.com/mochajs/mocha/wiki/Third-party-reporters

I have the following options:

1. I can remove the custom reporter and leave with the existing HTML reporter (the tests are running)
2. Run as a module in a node server with the cli (it will require a lot of changes the way the tests runs today)

Let me know if there's something maybe I'm missing and why this custom reporter is important

[JohnMcLear]

1. What's the existing HTML reporter User experience like Vs current solution? Screenshot would be nice. We need reporting to work w/ Travis as a side note.

[alexanmtz]

@JohnMcLear
I can see on the comments:

This reporter wraps the original HTML reporter plus reports plain text into a hidden div.
This allows the webdriver client to pick up the test results

This is the result with the HTML reporter:

It is a plain HTML, and I can include the mocha.css classes to give some style if we proceed with this solution.

[JohnMcLear]

So with this approach what's the actual difference to.

1. the person who runs the tests (other than some minor styling)
2. how tests run.
3. how tests are reporter (success/fail) to the runner > travis?

Everything looks the same (relatively speaking). What's the gotcha? :D

[alexanmtz]

Yeah @JohnMcLear, the trick is where these extended results in the plain text are used, is on Travis?

so taking out this extended reporter may break things, but I'm afraid that's the way to do it, otherwise, we will have more changes if we use the module implementation.

[JohnMcLear]

I don't know without looking at the runner logic how the results are passed from mocha > travis tbh ;\

[alexanmtz]

@JohnMcLear I can see the runner remote use the generated output from the custom reporter, so we need to keep it.

I should be able in the latest version to extend and use the custom reporter as we have today with some changes, but for some reason, I can't access Base from the Mocha instance to extend the reporter as you do today.

The current implementation extracted Base from the library in order to extend, but now this is not possible for this version.

So I opened this issue on Mocha repository:
mochajs/mocha#4259

So I would extend the reporter using the browser integration only if this is a bug or how we should do it, cause most of all examples out there use the module pattern, and here we have still the need to work in the old way, and it should be possible to keep the current implementation anyway.

[JohnMcLear]

Excellent work man, I really appreciate you handling this because I clearly was right in assuming I was out of my depths as far as hacking into Mocha :P

[JohnMcLear]

@alexanmtz where did you get to on this?

[alexanmtz]

@JohnMcLear nothing from the issue I created on Mocha.

So I will try another approach, using Mocha CLI that's is the right way to do in the latest version, and try to cause the minimum impact on the remote runners and at the current infrastructure, and the way the test runs today.

[alexanmtz]

I did a PR #4041 @JohnMcLear to check if is the right way to go 👍