Address feedback, cleanup tests

format + clippy

js/repl.ts

@@ -8,12 +8,12 @@ import * as dispatch from "./dispatch";
 import { exit } from "./os";
 import { window } from "./globals";
 
-function startRepl(name: string): number {
+function startRepl(historyFile: string): number {
   const builder = flatbuffers.createBuilder();
-  const name_ = builder.createString(name);
+  const historyFile_ = builder.createString(historyFile);
 
   msg.ReplStart.startReplStart(builder);
-  msg.ReplStart.addName(builder, name_);
+  msg.ReplStart.addHistoryFile(builder, historyFile_);
   const inner = msg.ReplStart.endReplStart(builder);
 
   const baseRes = dispatch.sendSync(builder, msg.Any.ReplStart, inner);
@@ -50,10 +50,10 @@ export function readline(rid: number, prompt: string): string {
 export function replLoop(): void {
   window.deno = deno; // FIXME use a new scope (rather than window).
 
-  const replName = "deno";
+  const historyFile = "deno_history.txt";
   const prompt = "> ";
 
-  const rid = startRepl(replName);
+  const rid = startRepl(historyFile);
 
   let line = "";
   while (true) {

src/deno_dir.rs

@@ -32,8 +32,6 @@ pub struct DenoDir {
   // This splits to http and https deps
   pub deps_http: PathBuf,
   pub deps_https: PathBuf,
-  // This is where repl data is stored, e.g. history.
-  pub repl: PathBuf,
   // If remote resources should be reloaded.
   reload: bool,
 }
@@ -57,7 +55,6 @@ impl DenoDir {
     let deps = root.as_path().join("deps");
     let deps_http = deps.join("http");
     let deps_https = deps.join("https");
-    let repl = root.as_path().join("repl");
 
     let deno_dir = DenoDir {
       root,
@@ -66,17 +63,14 @@ impl DenoDir {
       deps_http,
       deps_https,
       reload,
-      repl,
     };
     deno_fs::mkdir(deno_dir.gen.as_ref(), 0o755)?;
     deno_fs::mkdir(deno_dir.deps.as_ref(), 0o755)?;
     deno_fs::mkdir(deno_dir.deps_http.as_ref(), 0o755)?;
     deno_fs::mkdir(deno_dir.deps_https.as_ref(), 0o755)?;
-    deno_fs::mkdir(deno_dir.repl.as_ref(), 0o755)?;
 
     debug!("root {}", deno_dir.root.display());
     debug!("gen {}", deno_dir.gen.display());
-    debug!("repl {}", deno_dir.repl.display());
     debug!("deps {}", deno_dir.deps.display());
     debug!("deps_http {}", deno_dir.deps_http.display());
     debug!("deps_https {}", deno_dir.deps_https.display());

src/msg.fbs

@@ -278,7 +278,7 @@ table ReadlinkRes {
 }
 
 table ReplStart {
-  name: string;
+  history_file: string;
   // TODO add config
 }
 

src/ops.rs

@@ -20,6 +20,7 @@ use futures::Poll;
 use hyper;
 use hyper::rt::{Future, Stream};
 use remove_dir_all::remove_dir_all;
+use repl;
 use resources::table_entries;
 use std;
 use std::fs;
@@ -1088,7 +1089,6 @@ fn op_read_link(
   })
 }
 
-use repl::DenoRepl;
 fn op_repl_start(
   state: &Arc<IsolateState>,
   base: &msg::Base,
@@ -1097,20 +1097,17 @@ fn op_repl_start(
   assert_eq!(data.len(), 0);
   let inner = base.inner_as_repl_start().unwrap();
   let cmd_id = base.cmd_id();
-  let name = String::from(inner.name().unwrap());
+  let history_file = String::from(inner.history_file().unwrap());
 
-  debug!("op_repl_start {}", name);
-
-  let repl = DenoRepl::new(&name, &state.dir);
+  debug!("op_repl_start {}", history_file);
+  let history_path = repl::history_path(&state.dir, &history_file);
+  let repl = repl::DenoRepl::new(history_path);
   let resource = resources::add_repl(repl);
 
   let builder = &mut FlatBufferBuilder::new();
   let inner = msg::ReplStartRes::create(
     builder,
-    &msg::ReplStartResArgs {
-      rid: resource.rid,
-      ..Default::default()
-    },
+    &msg::ReplStartResArgs { rid: resource.rid },
   );
   ok_future(serialize_response(
     cmd_id,
@@ -1133,7 +1130,11 @@ fn op_repl_readline(
   let cmd_id = base.cmd_id();
   let rid = inner.rid();
   let prompt = inner.prompt().unwrap().to_owned();
+  debug!("op_repl_readline {} {}", rid, prompt);
 
+  // Ignore this clippy warning until this issue is addressed:
+  // https://github.com/rust-lang-nursery/rust-clippy/issues/1684
+  #[cfg_attr(feature = "cargo-clippy", allow(redundant_closure_call))]
   Box::new(futures::future::result((move || {
     let line = resources::readline(rid, &prompt)?;
 
@@ -1143,7 +1144,6 @@ fn op_repl_readline(
       builder,
       &msg::ReplReadlineResArgs {
         line: Some(line_off),
-        ..Default::default()
       },
     );
     Ok(serialize_response(

src/repl.rs

@@ -56,25 +56,23 @@ impl<T: rustyline::Helper> DerefMut for Editor<T> {
 }
 
 pub struct DenoRepl {
-  pub name: String,
   editor: Editor<()>,
   history_file: PathBuf,
 }
 
 impl DenoRepl {
-  pub fn new(name: &String, dir: &DenoDir) -> DenoRepl {
+  pub fn new(history_file: PathBuf) -> DenoRepl {
     let mut repl = DenoRepl {
-      name: name.clone(),
       editor: Editor::<()>::new(),
-      history_file: history_path(dir, name),
+      history_file,
     };
 
     repl.load_history();
     repl
   }
 
   fn load_history(&mut self) -> () {
-    debug!("Loading history file: {:?}", self.history_file);
+    debug!("Loading REPL history: {:?}", self.history_file);
     self
       .editor
       .load_history(&self.history_file.to_str().unwrap())
@@ -87,14 +85,14 @@ impl DenoRepl {
     self
       .editor
       .save_history(&self.history_file.to_str().unwrap())
-      .map(|_| debug!("Saved history file to: {:?}", self.history_file))
+      .map(|_| debug!("Saved REPL history to: {:?}", self.history_file))
       .map_err(|e| {
-        eprintln!("Unable to save history file: {:?} {}", self.history_file, e);
+        eprintln!("Unable to save REPL history: {:?} {}", self.history_file, e);
         deno_error(ErrorKind::Other, e.description().to_string())
       })
   }
 
-  pub fn readline(&mut self, prompt: &String) -> DenoResult<String> {
+  pub fn readline(&mut self, prompt: &str) -> DenoResult<String> {
     self
       .editor
       .readline(&prompt)
@@ -117,8 +115,8 @@ impl Drop for DenoRepl {
   }
 }
 
-fn history_path(dir: &DenoDir, name: &String) -> PathBuf {
-  let mut p: PathBuf = dir.repl.clone();
-  p.push(format!("{}_history.txt", name));
+pub fn history_path(dir: &DenoDir, history_file: &str) -> PathBuf {
+  let mut p: PathBuf = dir.root.clone();
+  p.push(history_file);
   p
 }

src/resources.rs

@@ -230,10 +230,7 @@ pub fn add_repl(repl: DenoRepl) -> Resource {
   Resource { rid }
 }
 
-pub fn readline(
-  rid: ResourceId,
-  prompt: &String, // TODO use &str for prompt
-) -> DenoResult<String> {
+pub fn readline(rid: ResourceId, prompt: &str) -> DenoResult<String> {
   let mut table = RESOURCE_TABLE.lock().unwrap();
   let maybe_repr = table.get_mut(&rid);
   match maybe_repr {

tools/repl_test.py

@@ -1,9 +1,10 @@
 # Copyright 2018 the Deno authors. All rights reserved. MIT license.
 import os
 from subprocess import PIPE, Popen
+import sys
 from time import sleep
 
-from util import build_path, executable_suffix
+from util import build_path, executable_suffix, green_ok
 
 
 class Repl(object):
@@ -27,92 +28,77 @@ def input(self, *lines, **kwargs):
             p.wait()
             raise
         retcode = p.poll()
-        return out, err, retcode
+        # Ignore Windows CRLF (\r\n).
+        return out.replace('\r\n', '\n'), err.replace('\r\n', '\n'), retcode
 
     def warm_up(self):
         # This may output an error message about the history file (ignore it).
         self.input("")
 
     def test_function(self):
-        print(" function")
         out, err, code = self.input("deno.writeFileSync")
         assertEqual(out, '[Function: writeFileSync]\n')
         assertEqual(err, '')
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_console_log(self):
-        print(" console_log")
         out, err, code = self.input("console.log('hello')", "'world'")
         assertEqual(out, 'hello\nundefined\nworld\n')
         assertEqual(err, '')
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_variable(self):
-        print(" variable")
         out, err, code = self.input("var a = 123;", "a")
         assertEqual(out, 'undefined\n123\n')
         assertEqual(err, '')
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_settimeout(self):
-        print(" settimeout")
         out, err, code = self.input(
             "setTimeout(() => { console.log('b'); deno.exit(0); }, 10)",
             "'a'",
             exit=False)
         assertEqual(out, '1\na\nb\n')
         assertEqual(err, '')
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_reference_error(self):
-        print(" reference_error")
         out, err, code = self.input("not_a_variable")
         assertEqual(out, '')
         assertEqual(err, 'ReferenceError: not_a_variable is not defined\n')
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_syntax_error(self):
-        print(" syntax_error")
         out, err, code = self.input("syntax error")
         assertEqual(out, '')
         assertEqual(err, "SyntaxError: Unexpected identifier\n")
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_type_error(self):
-        print(" type_error")
         out, err, code = self.input("console()")
         assertEqual(out, '')
         assertEqual(err, 'TypeError: console is not a function\n')
-        assert code == 0
+        assertEqual(code, 0)
 
     def test_exit_command(self):
-        print(" exit_command")
         out, err, code = self.input(".exit", "'ignored'", exit=False)
         assertEqual(out, '')
         assertEqual(err, '')
-        assert code == 0
+        assertEqual(code, 0)
 
     def run(self):
-        print('Running Repl tests...')
-        self.test_function()
-        self.test_console_log()
-        self.test_variable()
-        self.test_settimeout()
-        self.test_reference_error()
-        self.test_syntax_error()
-        self.test_type_error()
-        self.test_exit_command()
-        print('\nRepl tests successful.\n\n')
+        print('repl_test.py')
+        test_names = [name for name in dir(self) if name.startswith("test_")]
+        for t in test_names:
+            self.__getattribute__(t)()
+            sys.stdout.write(".")
+            sys.stdout.flush()
+        print(' {}\n'.format(green_ok()))
 
 
 def assertEqual(left, right):
-    try:
-        # In the str case ignore Windows' CRLF.
-        left_, right_ = left.replace('\r\n', '\n'), right.replace('\r\n', '\n')
-    except AttributeError:
-        left_, right_ = left, right
-    if left_ != right_:
-        raise AssertionError("{} != {}".format(repr(left_), repr(right_)))
+    if left != right:
+        raise AssertionError("{} != {}".format(repr(left), repr(right)))
 
 
 def repl_tests(deno_exe):