On Thu, 27 Feb 2020 15:56:25 GMT, Rony G. Flatscher
<github.com+60214806+rony...@openjdk.org> wrote:
>> …9: FXMLLoader does not populate ENGINE_SCOPE Bindings with FILENAME and ARGV
>
> Rony G. Flatscher has updated the pull request incrementally with one
> additional commit since the last revision:
>
> corrected wrong test string
The fix looks good. I left a few comments on the test. One of them is
substantive, the rest are formatting. Once you
make those changes, I'll approve it.
tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java
line 104:
> 103: ioe.printStackTrace();
> 104: System.exit(-1);
> 105: }
This should be `System.exit(ERROR_UNEXPECTED_EXCEPTION);`
tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java
line 185:
> 184:
> 185: // global Bindings
> 186: Util.assertExists(IDROOT + " in global scope Bindings",
> globalBindings.containsKey(IDROOT));
Minor: indentation is wrong (indented too far)
tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java
line 208:
> 207:
> 208: // engine Bindings
> 209: Util.assertExists(FILENAME + " in engine scope Bindings",
> engineBindings.containsKey(FILENAME));
Minor: indentation
tests/system/src/testscriptapp1/java/mymod/pseudoScriptEngine/InvocationInfos.java
line 55:
> 54: * @return string formatted to ease debugging
> 55: */
> 56: public String toDebugFormat(String indentation) {
Minor: the `/**` should be on a line by itself. Also, the closing `*/` needs
one more space of indentation.
tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java
line 109:
> 108: btn.fireEvent(new ActionEvent());
> 109: btn.fireEvent(new MouseEvent( MouseEvent.MOUSE_CLICKED,
> 110: 0, // double x,
Minor: remove the extra space after the last `(`
tests/system/src/testscriptapp1/java/mymod/myapp1/FXMLScriptDeployment.java
line 229:
> 228: }
> 229: else {
> 230: Util.assertType(EVENT, ActionEvent.class, obj);
Minor: the `}` should be on the same line as the `else {`
tests/system/src/testscriptapp1/java/mymod/pseudoScriptEngine/RgfPseudoScriptEngine.java
line 47:
> 46: public class RgfPseudoScriptEngine extends AbstractScriptEngine
> 47: {
> 48: static final boolean bDebug = false; // true;
The `{` should be on the previous line.
tests/system/src/testscriptapp1/java/mymod/pseudoScriptEngine/RgfPseudoScriptEngine.java
line 89:
> 88: // create copies of the Bindings for later inspection as they
> may
> 89: // get reused and changed on each eval() invocation
> 90: TreeMap<Integer,TreeMap> bindings = new TreeMap();
Minor: indentation
-------------
PR: https://git.openjdk.java.net/jfx/pull/122
