Rename and add test for HTMLFormatter #1218
Conversation
| @Test | ||
| public void writes_valid_report_js() throws Throwable { | ||
| @Test(expected = EcmaError.class) | ||
| public void empty_report_js_throws_error() throws Throwable { |
There was a problem hiding this comment.
Wasn't this this the test that caused the problems with the locale because the EcmaError contained a localized message?
There was a problem hiding this comment.
Yes it was. But I cannot reproduce that either by setting my laptop's locale or passing a VM argument (see PR description).
I can only reproduce it by explicitly setting the Locale in the test and then resetting after.
There was a problem hiding this comment.
As discussed, removed the test that checks for error and replaced with test that verifies content of reportJs.
| assertTrue(expected.getMessage().startsWith("ReferenceError: \"document\" is not defined.")); | ||
| } | ||
| String reportJs = FixJava.readReader(new InputStreamReader(new URL(outputDir, "report.js").openStream(), "UTF-8")); | ||
| assertEquals("$(document).ready(function() {var formatter = new CucumberHTML.DOMFormatter($('.cucumber-report'));formatter.uri(\"some\\\\windows\\\\path\\\\some.feature\");\n" + |
There was a problem hiding this comment.
@mlvandijk @mpkorstanje assertEquals cannot be used to compare things with json data structures in them, since json data structures should be considered equal also if the attribute order differs. Thus we got a build failure when this PR was merged: https://travis-ci.org/cucumber/cucumber-jvm/jobs/273568042.
It is now fixed in fb15952
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Rename and add test for HTMLFormatter
Details
Trying to duplicate / fix #729
Cannot reproduce this bug by setting Locale for my laptop, or running the test with VM options: -Duser.language=fr (or nl, for that matter). Only by explicitly setting the Locale in the test (see previous PR).
Motivation and Context
The original test does not validate that report.js is valid; it validates that an empty report throws a particular exception. Replaced it with a test that does check for valid Javascript instead.
How Has This Been Tested?
'mvn clean install' => Build Success
Screenshots (if appropriate):
N/A
Types of changes
Checklist: