8006584: improve variable handling in NashornScriptEngine
authorsundar
Sat, 19 Jan 2013 09:14:43 +0530
changeset 16182 06e8c712f6a3
parent 16181 f6a125580f62
child 16183 b9578bc4cd68
8006584: improve variable handling in NashornScriptEngine Reviewed-by: jlaskey, hannesw
nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java
nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
--- a/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Fri Jan 18 17:55:04 2013 +0530
+++ b/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Sat Jan 19 09:14:43 2013 +0530
@@ -28,15 +28,11 @@
 import static jdk.nashorn.internal.runtime.ECMAErrors.referenceError;
 import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
 import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
-import static jdk.nashorn.internal.runtime.linker.Lookup.MH;
 
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
-import java.lang.invoke.MethodHandle;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
 import java.net.URL;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -140,6 +136,8 @@
             }
             throw new RuntimeException(e);
         }
+
+        context.setBindings(new ScriptObjectMirror(global, global), ScriptContext.ENGINE_SCOPE);
     }
 
     @Override
@@ -156,23 +154,6 @@
         return evalImpl(script.toCharArray(), ctxt);
     }
 
-    /*
-     * FIXME: This is not exactly right. Script execution should actually
-     * put the variables in ENGINE_SCOPE bindings. But, it is difficult
-     * (not possible?) with the way ScriptObject is implemented. So, for now
-     * giving access to script variables by accessing it from "globals". This
-     * way at least engine.get("foo") will work whenever "foo" is a global var
-     * defined by eval'ed scripts.
-     */
-    @Override
-    public Object get(final String key) {
-        Object value = super.get(key);
-        if (value == null) {
-            value = ScriptObjectMirror.wrap(global.get(key), global);
-        }
-        return (value == UNDEFINED) ? null : value;
-    }
-
     @Override
     public ScriptEngineFactory getFactory() {
         return factory;
@@ -306,23 +287,6 @@
         }
 
         value = ScriptObjectMirror.unwrap(value, global);
-
-        if (value instanceof Method) {
-            final Method method = (Method) value;
-            final int    mods   = method.getModifiers();
-            if (Modifier.isStatic(mods) && Modifier.isPublic(mods)) {
-               value = MH.find((Method)value);
-            }
-        }
-
-        if (value instanceof MethodHandle) {
-            value = ((GlobalObject)global).newScriptFunction(name, (MethodHandle)value, null, false);
-        }
-
-        if (!(value instanceof ScriptFunction)) {
-            typeError(global, "not.a.function", name);
-        }
-
         if (value instanceof ScriptFunction) {
             return ScriptObjectMirror.unwrap(ScriptRuntime.apply((ScriptFunction)value, global, args), global);
         }
@@ -359,13 +323,12 @@
     // scripts should see "context" and "engine" as variables
     private void setContextVariables(final ScriptContext ctxt) {
         ctxt.setAttribute("context", ctxt, ScriptContext.ENGINE_SCOPE);
-        // current ScriptContext exposed as "context"
         global.set("context", ctxt, false);
-        Object args = ctxt.getAttribute("arguments");
-        // if no arguments passed, make it empty array
-        if (args == null) {
+        Object args = ScriptObjectMirror.unwrap(ctxt.getAttribute("arguments"), global);
+        if (args == null || args == UNDEFINED) {
             args = ScriptRuntime.EMPTY_ARRAY;
         }
+        // if no arguments passed, expose it
         args = ((GlobalObject)global).wrapAsObject(args);
         global.set("arguments", args, false);
     }
@@ -382,7 +345,7 @@
             }
 
             ScriptObject sobj;
-            Object       value;
+            Object       value = null;
 
             self = ScriptObjectMirror.unwrap(self, global);
 
@@ -394,25 +357,6 @@
                 self  = global;
                 sobj  = global;
                 value = sobj.get(name);
-            } else {
-                // Find the java method and make a ScriptFunction of it.
-                final Method[] methods = self.getClass().getMethods();
-                Method target = null;
-
-                for (final Method mth : methods) {
-                    // choose the right overload by number of arguments -- don't
-                    // care overload resolution for now..
-                    if (mth.getName().equals(name) &&
-                        mth.getParameterTypes().length == args.length) {
-                        target = mth;
-                        break;
-                    }
-                }
-                if (target == null) {
-                    throw new NoSuchMethodException(name);
-                }
-                final GlobalObject gobj = (GlobalObject) global;
-                value = gobj.newScriptFunction(name, MH.find(target), null, false);
             }
 
             if (value instanceof ScriptFunction) {
@@ -423,7 +367,7 @@
                     throwAsScriptException(e);
                     throw new AssertionError("should not reach here");
                 }
-                return ScriptObjectMirror.wrap(res, global);
+                return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(res, global));
             }
 
             throw new NoSuchMethodException(name);
@@ -461,8 +405,7 @@
             }
 
             Object res = ScriptRuntime.apply(script, global);
-            res = ScriptObjectMirror.wrap(res, global);
-            return (res == UNDEFINED) ? null : res;
+            return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(res, global));
         } catch (final Exception e) {
             throwAsScriptException(e);
             throw new AssertionError("should not reach here");
@@ -516,7 +459,6 @@
                 setNashornGlobal(global);
             }
 
-            setContextVariables(ctxt);
             return nashornContext.compileScript(source, global, nashornContext._strict);
         } catch (final Exception e) {
             throwAsScriptException(e);
--- a/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Fri Jan 18 17:55:04 2013 +0530
+++ b/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Sat Jan 19 09:14:43 2013 +0530
@@ -35,6 +35,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
+import javax.script.Bindings;
 import jdk.nashorn.internal.runtime.Context;
 import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
@@ -43,9 +44,10 @@
 
 /**
  * Mirror object that wraps a given ScriptObject instance. User can
- * access ScriptObject via the java.util.Map interface.
+ * access ScriptObject via the javax.script.Bindings interface or
+ * netscape.javascript.JSObject interface.
  */
-final class ScriptObjectMirror extends JSObject implements Map<Object, Object> {
+final class ScriptObjectMirror extends JSObject implements Bindings {
     private final ScriptObject sobj;
     private final ScriptObject global;
 
@@ -146,12 +148,20 @@
 
     @Override
     public Object getMember(final String name) {
-        return get(name);
+        return inGlobal(new Callable<Object>() {
+            @Override public Object call() {
+                return wrap(sobj.get(name), global);
+            }
+        });
     }
 
     @Override
     public Object getSlot(final int index) {
-        return get(Integer.valueOf(index));
+        return inGlobal(new Callable<Object>() {
+            @Override public Object call() {
+                return wrap(sobj.get(index), global);
+            }
+        });
     }
 
     @Override
@@ -166,7 +176,12 @@
 
     @Override
     public void setSlot(final int index, final Object value) {
-        put(Integer.valueOf(index), wrap(value, Context.getGlobal()));
+        inGlobal(new Callable<Void>() {
+            @Override public Void call() {
+                sobj.set(index, unwrap(value, global), global.getContext()._strict);
+                return null;
+            }
+        });
     }
 
     @Override
@@ -175,7 +190,8 @@
             @Override public Object call() {
                 sobj.clear();
                 return null;
-            }});
+            }
+        });
     }
 
     @Override
@@ -183,7 +199,8 @@
         return inGlobal(new Callable<Boolean>() {
             @Override public Boolean call() {
                 return sobj.containsKey(unwrap(key, global));
-            }});
+            }
+        });
     }
 
     @Override
@@ -191,19 +208,20 @@
         return inGlobal(new Callable<Boolean>() {
             @Override public Boolean call() {
                 return sobj.containsValue(unwrap(value, global));
-            }});
+            }
+        });
     }
 
     @Override
-    public Set<Map.Entry<Object, Object>> entrySet() {
-        return inGlobal(new Callable<Set<Map.Entry<Object, Object>>>() {
-            @Override public Set<Map.Entry<Object, Object>> call() {
+    public Set<Map.Entry<String, Object>> entrySet() {
+        return inGlobal(new Callable<Set<Map.Entry<String, Object>>>() {
+            @Override public Set<Map.Entry<String, Object>> call() {
                 final Iterator<String>               iter    = sobj.propertyIterator();
-                final Set<Map.Entry<Object, Object>> entries = new HashSet<>();
+                final Set<Map.Entry<String, Object>> entries = new HashSet<>();
 
                 while (iter.hasNext()) {
-                    final Object key   = wrap(iter.next(), global);
-                    final Object value = wrap(sobj.get(key), global);
+                    final String key   = iter.next();
+                    final Object value = translateUndefined(wrap(sobj.get(key), global));
                     entries.add(new AbstractMap.SimpleImmutableEntry<>(key, value));
                 }
 
@@ -214,49 +232,58 @@
 
     @Override
     public Object get(final Object key) {
-        return inGlobal(new Callable<Object>() { @Override public Object call() {
-            return wrap(sobj.get(key), global);
-        }});
+        return inGlobal(new Callable<Object>() {
+            @Override public Object call() {
+                return translateUndefined(wrap(sobj.get(key), global));
+            }
+        });
     }
 
     @Override
     public boolean isEmpty() {
-        return inGlobal(new Callable<Boolean>() { @Override public Boolean call() {
-            return sobj.isEmpty();
-        }});
+        return inGlobal(new Callable<Boolean>() {
+            @Override public Boolean call() {
+                return sobj.isEmpty();
+            }
+        });
     }
 
     @Override
-    public Set<Object> keySet() {
-        return inGlobal(new Callable<Set<Object>>() { @Override public Set<Object> call() {
-            final Iterator<String> iter   = sobj.propertyIterator();
-            final Set<Object>      keySet = new HashSet<>();
+    public Set<String> keySet() {
+        return inGlobal(new Callable<Set<String>>() {
+            @Override public Set<String> call() {
+                final Iterator<String> iter   = sobj.propertyIterator();
+                final Set<String>      keySet = new HashSet<>();
 
-            while (iter.hasNext()) {
-                keySet.add(wrap(iter.next(), global));
+                while (iter.hasNext()) {
+                    keySet.add(iter.next());
+                }
+
+                return Collections.unmodifiableSet(keySet);
             }
-
-            return Collections.unmodifiableSet(keySet);
-        }});
+        });
     }
 
     @Override
-    public Object put(final Object key, final Object value) {
+    public Object put(final String key, final Object value) {
         return inGlobal(new Callable<Object>() {
             @Override public Object call() {
-                return sobj.put(unwrap(key, global), unwrap(value, global));
-        }});
+                return sobj.put(key, unwrap(value, global));
+            }
+        });
     }
 
     @Override
-    public void putAll(final Map<?, ?> map) {
+    public void putAll(final Map<? extends String, ? extends Object> map) {
         final boolean strict = sobj.getContext()._strict;
-        inGlobal(new Callable<Object>() { @Override public Object call() {
-            for (final Map.Entry<?, ?> entry : map.entrySet()) {
-                sobj.set(unwrap(entry.getKey(), global), unwrap(entry.getValue(), global), strict);
+        inGlobal(new Callable<Object>() {
+            @Override public Object call() {
+                for (final Map.Entry<? extends String, ? extends Object> entry : map.entrySet()) {
+                    sobj.set(entry.getKey(), unwrap(entry.getValue(), global), strict);
+                }
+                return null;
             }
-            return null;
-        }});
+        });
     }
 
     @Override
@@ -279,16 +306,22 @@
 
     @Override
     public Collection<Object> values() {
-        return inGlobal(new Callable<Collection<Object>>() { @Override public Collection<Object> call() {
-            final List<Object>     values = new ArrayList<>(size());
-            final Iterator<Object> iter   = sobj.valueIterator();
+        return inGlobal(new Callable<Collection<Object>>() {
+            @Override public Collection<Object> call() {
+                final List<Object>     values = new ArrayList<>(size());
+                final Iterator<Object> iter   = sobj.valueIterator();
 
-            while (iter.hasNext()) {
-                values.add(wrap(iter.next(), global));
+                while (iter.hasNext()) {
+                    values.add(wrap(iter.next(), global));
+                }
+
+                return Collections.unmodifiableList(values);
             }
+        });
+    }
 
-            return Collections.unmodifiableList(values);
-        }});
+    static Object translateUndefined(Object obj) {
+        return (obj == ScriptRuntime.UNDEFINED)? null : obj;
     }
 
     static Object wrap(final Object obj, final ScriptObject homeGlobal) {
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Fri Jan 18 17:55:04 2013 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Sat Jan 19 09:14:43 2013 +0530
@@ -27,6 +27,7 @@
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
@@ -40,13 +41,13 @@
 import javax.script.Compilable;
 import javax.script.CompiledScript;
 import javax.script.Invocable;
+import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineFactory;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
 import jdk.nashorn.internal.runtime.Version;
 import netscape.javascript.JSObject;
-import org.testng.Assert;
 import org.testng.TestNG;
 import org.testng.annotations.Test;
 
@@ -107,8 +108,8 @@
         final ScriptEngine e = m.getEngineByName("nashorn");
 
         try {
-            assertEquals(true,e.eval("arguments instanceof Array"));
-            assertEquals(true, e.eval("arguments.length == 0"));
+            assertEquals(e.eval("arguments instanceof Array"), true);
+            assertEquals(e.eval("arguments.length == 0"), true);
         } catch (final Exception exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -123,13 +124,13 @@
 
         final ScriptEngineFactory fac = e.getFactory();
 
-        assertEquals("ECMAScript", fac.getLanguageName());
-        assertEquals("javascript", fac.getParameter(ScriptEngine.NAME));
-        assertEquals("ECMA - 262 Edition 5.1", fac.getLanguageVersion());
-        assertEquals("Oracle Nashorn", fac.getEngineName());
-        assertEquals(Version.version(), fac.getEngineVersion());
-        assertEquals("print(context)", fac.getOutputStatement("context"));
-        assertEquals("javascript", fac.getParameter(ScriptEngine.NAME));
+        assertEquals(fac.getLanguageName(), "ECMAScript");
+        assertEquals(fac.getParameter(ScriptEngine.NAME), "javascript");
+        assertEquals(fac.getLanguageVersion(), "ECMA - 262 Edition 5.1");
+        assertEquals(fac.getEngineName(), "Oracle Nashorn");
+        assertEquals(fac.getEngineVersion(), Version.version());
+        assertEquals(fac.getOutputStatement("context"), "print(context)");
+        assertEquals(fac.getParameter(ScriptEngine.NAME), "javascript");
 
         boolean seenJS = false;
         for (String ext : fac.getExtensions()) {
@@ -138,9 +139,9 @@
             }
         }
 
-        assertEquals(true, seenJS);
+        assertEquals(seenJS, true);
         String str = fac.getMethodCallSyntax("obj", "foo", "x");
-        assertEquals("obj.foo(x)", str);
+        assertEquals(str, "obj.foo(x)");
 
         boolean seenNashorn = false, seenJavaScript = false, seenECMAScript = false;
         for (String name : fac.getNames()) {
@@ -151,9 +152,9 @@
             }
         }
 
-        assertEquals(true, seenNashorn);
-        assertEquals(true, seenJavaScript);
-        assertEquals(true, seenECMAScript);
+        assertTrue(seenNashorn);
+        assertTrue(seenJavaScript);
+        assertTrue(seenECMAScript);
 
         boolean seenAppJS = false, seenAppECMA = false, seenTextJS = false, seenTextECMA = false;
         for (String mime : fac.getMimeTypes()) {
@@ -165,10 +166,10 @@
             }
         }
 
-        assertEquals(true, seenAppJS);
-        assertEquals(true, seenAppECMA);
-        assertEquals(true, seenTextJS);
-        assertEquals(true, seenTextECMA);
+        assertTrue(seenAppJS);
+        assertTrue(seenAppECMA);
+        assertTrue(seenTextJS);
+        assertTrue(seenTextECMA);
     }
 
     @Test
@@ -186,9 +187,9 @@
             e.eval("print('hello)");
             fail("script exception expected");
         } catch (final ScriptException se) {
-            assertEquals(1, se.getLineNumber());
-            assertEquals(13, se.getColumnNumber());
-            assertEquals("myfile.js", se.getFileName());
+            assertEquals(se.getLineNumber(), 1);
+            assertEquals(se.getColumnNumber(), 13);
+            assertEquals(se.getFileName(), "myfile.js");
             // se.printStackTrace();
         }
 
@@ -251,7 +252,7 @@
             fail(se.getMessage());
         }
 
-        assertEquals(Boolean.TRUE, res);
+        assertEquals(res, Boolean.TRUE);
     }
 
     @Test
@@ -289,7 +290,7 @@
 
         try {
             e.eval("var x = 'hello'");
-            assertEquals("hello", e.get("x"));
+            assertEquals(e.get("x"), "hello");
         } catch (final ScriptException exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -310,21 +311,21 @@
         }
     }
 
-    public static void alert(final Object self, final Object msg) {
+    public static void alert(final Object msg) {
         System.out.println(msg);
     }
 
     @Test
-    public void exposeFunctionTest() {
+    public void exposeMethodTest() {
         final ScriptEngineManager m = new ScriptEngineManager();
         final ScriptEngine e = m.getEngineByName("nashorn");
 
         try {
-            final Method alert = ScriptEngineTest.class.getMethod("alert", Object.class, Object.class);
+            final Method alert = ScriptEngineTest.class.getMethod("alert", Object.class);
             // expose a Method object as global var.
             e.put("alert", alert);
             // call the global var.
-            e.eval("alert('alert! alert!!')");
+            e.eval("alert.invoke(null, 'alert! alert!!')");
         } catch (final NoSuchMethodException | SecurityException | ScriptException exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -359,11 +360,8 @@
 
         try {
             e.put("window", window);
-            e.eval("print(window.alert)"); // TODO: bug - prints 'undefined'
+            e.eval("print(window.alert)");
             e.eval("window.alert('calling window.alert...')");
-            // TODO: java.lang.NoSuchMethodException: alert
-            // ((Invocable) e).invokeMethod(window, "alert",
-            // "invoking window.alert...");
         } catch (final Exception exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -379,8 +377,8 @@
         try {
             e.put("window", window);
             e.eval("print(window.location)");
-            final Object locationValue = ((Invocable)e).invokeMethod(window, "getLocation");
-            assertEquals("http://localhost:8080/window", locationValue);
+            final Object locationValue = e.eval("window.getLocation()");
+            assertEquals(locationValue, "http://localhost:8080/window");
         } catch (final Exception exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -396,9 +394,9 @@
         try {
             e.put("window", window);
             final String item1 = (String)e.eval("window.item(65535)");
-            assertEquals("ffff", item1);
-            final String item2 = (String)((Invocable)e).invokeMethod(window, "item", 255);
-            assertEquals("ff", item2);
+            assertEquals(item1, "ffff");
+            final String item2 = (String)e.eval("window.item(255)");
+            assertEquals(item2, "ff");
         } catch (final Exception exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -434,9 +432,9 @@
             e.eval("throw 'foo'");
         } catch (final ScriptException exp) {
             log(exp.getMessage());
-            assertEquals("foo in throwtest.js at line number 1 at column number 0", exp.getMessage());
-            assertEquals("throwtest.js", exp.getFileName());
-            assertEquals(1, exp.getLineNumber());
+            assertEquals(exp.getMessage(), "foo in throwtest.js at line number 1 at column number 0");
+            assertEquals(exp.getFileName(), "throwtest.js");
+            assertEquals(exp.getLineNumber(), 1);
         }
     }
 
@@ -491,18 +489,18 @@
 
         int count = 0;
         Map<Object, Object> map = (Map<Object, Object>)e.get("obj");
-        assertEquals(false, map.isEmpty());
-        assertEquals(true, map.keySet().contains("x"));
-        assertEquals(true, map.containsKey("x"));
-        assertEquals(true, map.values().contains("nashorn"));
-        assertEquals(true, map.containsValue("nashorn"));
+        assertFalse(map.isEmpty());
+        assertTrue(map.keySet().contains("x"));
+        assertTrue(map.containsKey("x"));
+        assertTrue(map.values().contains("nashorn"));
+        assertTrue(map.containsValue("nashorn"));
         for (final Map.Entry<?, ?> ex : map.entrySet()) {
             final Object key = ex.getKey();
             if (key.equals("x")) {
                 assertTrue(344 == ((Number)ex.getValue()).doubleValue());
                 count++;
             } else if (key.equals("y")) {
-                assertEquals("nashorn", ex.getValue());
+                assertEquals(ex.getValue(), "nashorn");
                 count++;
             }
         }
@@ -511,56 +509,56 @@
 
         // add property
         map.put("z", "hello");
-        assertEquals("hello", e.eval("obj.z"));
-        assertEquals("hello", map.get("z"));
-        assertEquals(true, map.keySet().contains("z"));
-        assertEquals(true, map.containsKey("z"));
-        assertEquals(true, map.values().contains("hello"));
-        assertEquals(true, map.containsValue("hello"));
-        assertEquals(3, map.size());
+        assertEquals(e.eval("obj.z"), "hello");
+        assertEquals(map.get("z"), "hello");
+        assertTrue(map.keySet().contains("z"));
+        assertTrue(map.containsKey("z"));
+        assertTrue(map.values().contains("hello"));
+        assertTrue(map.containsValue("hello"));
+        assertEquals(map.size(), 3);
 
         final Map<Object, Object> newMap = new HashMap<>();
         newMap.put("foo", 23.0);
         newMap.put("bar", true);
         map.putAll(newMap);
 
-        assertEquals(23.0, e.eval("obj.foo"));
-        assertEquals(true, e.eval("obj.bar"));
+        assertEquals(e.eval("obj.foo"), 23.0);
+        assertEquals(e.eval("obj.bar"), true);
 
         // remove using map method
         map.remove("foo");
-        assertEquals("undefined", e.eval("typeof obj.foo"));
+        assertEquals(e.eval("typeof obj.foo"), "undefined");
 
         count = 0;
         e.eval("var arr = [ true, 'hello' ]");
         map = (Map<Object, Object>)e.get("arr");
-        assertEquals(false, map.isEmpty());
-        assertEquals(true, map.containsKey("length"));
-        assertEquals(true, map.containsValue("hello"));
+        assertFalse(map.isEmpty());
+        assertTrue(map.containsKey("length"));
+        assertTrue(map.containsValue("hello"));
         for (final Map.Entry<?, ?> ex : map.entrySet()) {
             final Object key = ex.getKey();
             if (key.equals("0")) {
-                assertEquals(Boolean.TRUE, ex.getValue());
+                assertEquals(ex.getValue(), Boolean.TRUE);
                 count++;
             } else if (key.equals("1")) {
-                assertEquals("hello", ex.getValue());
+                assertEquals(ex.getValue(), "hello");
                 count++;
             }
         }
-        assertEquals(2, count);
-        assertEquals(2, map.size());
+        assertEquals(count, 2);
+        assertEquals(map.size(), 2);
 
         // add element
-        map.put(2, "world");
-        assertEquals("world", map.get(2));
-        assertEquals(3, map.size());
+        map.put("2", "world");
+        assertEquals(map.get("2"), "world");
+        assertEquals(map.size(), 3);
 
         // remove all
         map.clear();
-        assertEquals(true, map.isEmpty());
-        assertEquals("undefined", e.eval("typeof arr[0]"));
-        assertEquals("undefined", e.eval("typeof arr[1]"));
-        assertEquals("undefined", e.eval("typeof arr[2]"));
+        assertTrue(map.isEmpty());
+        assertEquals(e.eval("typeof arr[0]"), "undefined");
+        assertEquals(e.eval("typeof arr[1]"), "undefined");
+        assertEquals(e.eval("typeof arr[2]"), "undefined");
     }
 
     @Test
@@ -585,7 +583,7 @@
             e.eval("var Example = function() { this.hello = function() { return 'Hello World!'; };}; myExample = new Example();");
             final Object obj = e.get("myExample");
             final Object res = ((Invocable)e).invokeMethod(obj, "hello");
-            assertEquals("Hello World!", res);
+            assertEquals(res, "Hello World!");
         } catch (final Exception exp) {
             exp.printStackTrace();
             fail(exp.getMessage());
@@ -867,7 +865,7 @@
         final ScriptEngine e = m.getEngineByName("nashorn");
         try {
             Object obj = e.eval("new TypeError('wrong type')");
-            Assert.assertEquals(obj.toString(), "TypeError: wrong type", "toString returns wrong value");
+            assertEquals(obj.toString(), "TypeError: wrong type", "toString returns wrong value");
         } catch (final Throwable t) {
             t.printStackTrace();
             fail(t.getMessage());
@@ -875,10 +873,53 @@
 
         try {
             Object obj = e.eval("function func() { print('hello'); }");
-            Assert.assertEquals(obj.toString(), "function func() { print('hello'); }", "toString returns wrong value");
+            assertEquals(obj.toString(), "function func() { print('hello'); }", "toString returns wrong value");
         } catch (final Throwable t) {
             t.printStackTrace();
             fail(t.getMessage());
         }
     }
+
+    @Test
+    public void engineScopeTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+        Bindings engineScope = e.getBindings(ScriptContext.ENGINE_SCOPE);
+
+        // check few ECMA standard built-in global properties
+        assertNotNull(engineScope.get("Object"));
+        assertNotNull(engineScope.get("TypeError"));
+        assertNotNull(engineScope.get("eval"));
+
+        // can access via ScriptEngine.get as well
+        assertNotNull(e.get("Object"));
+        assertNotNull(e.get("TypeError"));
+        assertNotNull(e.get("eval"));
+
+        // Access by either way should return same object
+        assertEquals(engineScope.get("Array"), e.get("Array"));
+        assertEquals(engineScope.get("EvalError"), e.get("EvalError"));
+        assertEquals(engineScope.get("undefined"), e.get("undefined"));
+
+        // try exposing a new variable from scope
+        engineScope.put("myVar", "foo");
+        try {
+            assertEquals(e.eval("myVar"), "foo");
+        } catch (final ScriptException se) {
+            se.printStackTrace();
+            fail(se.getMessage());
+        }
+
+        // update "myVar" in script an check the value from scope
+        try {
+            e.eval("myVar = 'nashorn';");
+        } catch (final ScriptException se) {
+            se.printStackTrace();
+            fail(se.getMessage());
+        }
+
+        // now check modified value from scope and engine
+        assertEquals(engineScope.get("myVar"), "nashorn");
+        assertEquals(e.get("myVar"), "nashorn");
+    }
 }