From d279437af342c399f3d4bf1d6c16c324186be8aa Mon Sep 17 00:00:00 2001
From: Jon Jagger <jon@jaggersoft.com>
Date: Mon, 19 Dec 2016 21:54:28 +0000
Subject: [PATCH] removed storer.kata_exists() and storer.avatar_exists()
 methods; kata_controller.run_tests() now does one less service round-trip

---
 app/controllers/forker_controller.rb          | 22 ++++++++-
 app/controllers/kata_controller.rb            |  2 -
 .../setup_custom_start_point_controller.rb    |  3 +-
 .../setup_default_start_point_controller.rb   |  6 ++-
 app/lib/fake_storer.rb                        | 41 ++++++++--------
 app/lib/storer_service.rb                     | 17 -------
 app/models/avatar.rb                          |  2 +-
 app/models/katas.rb                           |  2 +-
 test.sh                                       |  5 +-
 test/app_lib/fake_storer_test.rb              | 48 -------------------
 test/app_lib/storer_service_test.rb           |  4 --
 test/app_models/katas_test.rb                 | 10 ++--
 test/print_coverage_summary.rb                |  2 +-
 13 files changed, 56 insertions(+), 108 deletions(-)

diff --git a/app/controllers/forker_controller.rb b/app/controllers/forker_controller.rb
index a7268fe39..8dea7905a 100644
--- a/app/controllers/forker_controller.rb
+++ b/app/controllers/forker_controller.rb
@@ -5,12 +5,12 @@ def fork
     result = { forked: false }
     error = false
 
-    if !error && kata.nil?
+    if !error && bad_kata_id?
       error = true
       result[:reason] = "dojo(#{id})"
     end
 
-    if !error && avatar.nil?
+    if !error && bad_avatar_name?
       error = true
       result[:reason] = "avatar(#{avatar_name})"
     end
@@ -71,5 +71,23 @@ def fork
   include TimeNow
   include UniqueId
 
+  def bad_kata_id?
+    begin
+      kata.created
+      return false
+    rescue StandardError
+      return true
+    end
+  end
+
+  def bad_avatar_name?
+    begin
+      avatar.lights
+      return false
+    rescue StandardError
+      return true
+    end
+  end
+
 end
 
diff --git a/app/controllers/kata_controller.rb b/app/controllers/kata_controller.rb
index 305455ef7..ce0c737c0 100644
--- a/app/controllers/kata_controller.rb
+++ b/app/controllers/kata_controller.rb
@@ -12,9 +12,7 @@ def edit
   end
 
   def run_tests
-    fail "sorry, we can't do that" if kata.nil? || avatar.nil?
     @avatar = avatar
-
     incoming = params[:file_hashes_incoming]
     outgoing = params[:file_hashes_outgoing]
     delta = FileDeltaMaker.make_delta(incoming, outgoing)
diff --git a/app/controllers/setup_custom_start_point_controller.rb b/app/controllers/setup_custom_start_point_controller.rb
index 6dd40b79a..e285ebfad 100644
--- a/app/controllers/setup_custom_start_point_controller.rb
+++ b/app/controllers/setup_custom_start_point_controller.rb
@@ -7,7 +7,8 @@ def show
     @id = id
     @title = 'create'
     custom_names = display_names_of(dojo.custom)
-    index = choose_language(custom_names, dojo.katas[id])
+    kata = (id != nil) ? dojo.katas[id] : nil
+    index = choose_language(custom_names, kata)
     @start_points = ::DisplayNamesSplitter.new(custom_names, index)
   end
 
diff --git a/app/controllers/setup_default_start_point_controller.rb b/app/controllers/setup_default_start_point_controller.rb
index f54452a02..d11282f22 100644
--- a/app/controllers/setup_default_start_point_controller.rb
+++ b/app/controllers/setup_default_start_point_controller.rb
@@ -9,7 +9,8 @@ def show_languages
     @id = id
     @title = 'create'
     languages_names = display_names_of(languages)
-    index = choose_language(languages_names, dojo.katas[id])
+    kata = (id != nil) ? dojo.katas[id] : nil
+    index = choose_language(languages_names, kata)
     @start_points = ::DisplayNamesSplitter.new(languages_names, index)
     @max_seconds = runner.max_seconds
   end
@@ -20,7 +21,8 @@ def show_exercises
     @language = params[:language]
     @test = params[:test]
     @exercises_names,@exercises = read_exercises
-    @initial_index = choose_exercise(@exercises_names, dojo.katas[id])
+    kata = (id != nil) ? dojo.katas[id] : nil
+    @initial_index = choose_exercise(@exercises_names, kata)
   end
 
   def save
diff --git a/app/lib/fake_storer.rb b/app/lib/fake_storer.rb
index 8083797e6..09efc5552 100644
--- a/app/lib/fake_storer.rb
+++ b/app/lib/fake_storer.rb
@@ -38,10 +38,6 @@ def completions(outer_dir)
 
   # - - - - - - - - - - - - - - - - - - - - - - - -
 
-  def kata_exists?(id)
-    valid?(id) && kata_dir(id).exists?
-  end
-
   def create_kata(manifest)
     dir = kata_dir(manifest[:id])
     dir.make
@@ -73,10 +69,6 @@ def kata_start_avatar(id, avatar_names)
 
   # - - - - - - - - - - - - - - - - - - - - - - - -
 
-  def avatar_exists?(id, name)
-    avatar_dir(id, name).exists?
-  end
-
   def avatar_increments(id, name)
     [tag0(id)] + increments(id, name)
   end
@@ -160,14 +152,6 @@ def increments(id, name)
 
   # - - - - - - - - - - - - - - - -
 
-  def valid?(id)
-    id.class.name == 'String' &&
-      id.length == 10 &&
-        id.chars.all? { |char| hex?(char) }
-  end
-
-  # - - - - - - - - - - - - - - - -
-
   def write_avatar_increments(id, name, increments)
     dir = avatar_dir(id, name)
     dir.write_json(increments_filename, increments)
@@ -181,10 +165,6 @@ def write_tag_manifest(id, name, tag, files)
 
   # - - - - - - - - - - - - - - - -
 
-  def hex?(char)
-    '0123456789ABCDEF'.include?(char)
-  end
-
   def increments_filename
     'increments.json'
   end
@@ -193,7 +173,24 @@ def manifest_filename
     'manifest.json'
   end
 
-  include NearestAncestors
-  def env_var; nearest_ancestors(:env_var); end
+  # - - - - - - - - - - - - - - - -
+
+  def valid?(id)
+    id.class.name == 'String' &&
+      id.length == 10 &&
+        id.chars.all? { |char| hex?(char) }
+  end
+
+  def hex?(char)
+    '0123456789ABCDEF'.include?(char)
+  end
+
+  def kata_exists?(id)
+    valid?(id) && kata_dir(id).exists?
+  end
+
+  def avatar_exists?(id, name)
+    avatar_dir(id, name).exists?
+  end
 
 end
diff --git a/app/lib/storer_service.rb b/app/lib/storer_service.rb
index 4b48f2089..db872ace8 100644
--- a/app/lib/storer_service.rb
+++ b/app/lib/storer_service.rb
@@ -11,15 +11,6 @@ def initialize(parent)
 
   # - - - - - - - - - - - -
 
-  def kata_exists?(kata_id)
-    kata_exists(kata_id)
-  end
-  def avatar_exists?(kata_id, avatar_name)
-    avatar_exists(kata_id, avatar_name)
-  end
-
-  # - - - - - - - - - - - -
-
   def path
     get(__method__)
   end
@@ -42,10 +33,6 @@ def completions(id)
 
   # - - - - - - - - - - - -
 
-  def kata_exists(kata_id)
-    get(__method__, kata_id)
-  end
-
   def kata_manifest(kata_id)
     get(__method__, kata_id)
   end
@@ -60,10 +47,6 @@ def kata_started_avatars(kata_id)
 
   # - - - - - - - - - - - -
 
-  def avatar_exists(kata_id, avatar_name)
-    get(__method__, kata_id, avatar_name)
-  end
-
   def avatar_increments(kata_id, avatar_name)
     get(__method__, kata_id, avatar_name)
   end
diff --git a/app/models/avatar.rb b/app/models/avatar.rb
index cae771f36..cb3c4e543 100644
--- a/app/models/avatar.rb
+++ b/app/models/avatar.rb
@@ -47,7 +47,7 @@ def active?
     # instructions. I don't want these avatars appearing on the
     # dashboard. When forking a new kata you can enter as one
     # animal to sanity check it is ok (but not press [test])
-    storer.avatar_exists?(kata.id, name) && !lights.empty?
+    !lights.empty?
   end
 
   def tags
diff --git a/app/models/katas.rb b/app/models/katas.rb
index d0e948061..d9b0c050f 100644
--- a/app/models/katas.rb
+++ b/app/models/katas.rb
@@ -23,7 +23,7 @@ def completed(id)
   end
 
   def [](id)
-    storer.kata_exists?(id) ? Kata.new(self, id) : nil
+    Kata.new(self, id)
   end
 
   # modifiers
diff --git a/test.sh b/test.sh
index 955bd6b5e..a3c204249 100755
--- a/test.sh
+++ b/test.sh
@@ -1,11 +1,12 @@
 #!/bin/bash
-set -e
+#Don't do [set -e] because we want to get coverage stats out
 
 storer_cid=`docker ps --all --quiet --filter "name=cyber-dojo-storer"`
 docker exec ${storer_cid} sh -c "rm -rf /tmp/cyber-dojo/katas/*"
 
 web_cid=`docker ps --all --quiet --filter "name=cyber-dojo-web"`
 docker exec ${web_cid} sh -c "cd test && ./run.sh ${*}"
+status=$?
 
 # copy coverage stats out of container
 my_dir="$( cd "$( dirname "${0}" )" && pwd )"
@@ -14,3 +15,5 @@ mkdir -p ${my_dir}/coverage
 src=${web_cid}:/tmp/cyber-dojo/coverage/.
 dst=${my_dir}/coverage/
 docker cp ${src} ${dst}
+
+exit ${status}
\ No newline at end of file
diff --git a/test/app_lib/fake_storer_test.rb b/test/app_lib/fake_storer_test.rb
index c3148df41..69ca088b1 100755
--- a/test/app_lib/fake_storer_test.rb
+++ b/test/app_lib/fake_storer_test.rb
@@ -26,7 +26,6 @@ def storer
   'but symbol-keys have become string-keys' do
     manifest = make_manifest(kata_id = '603E8BAEDF')
     storer.create_kata(manifest)
-    assert kata_exists?(kata_id)
     expected = manifest
     actual = kata_manifest(kata_id)
     assert_equal expected.keys.size, actual.keys.size
@@ -35,39 +34,6 @@ def storer
     end
   end
 
-  #- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-  # kata_exists?(id)
-  #- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-
-  test '9D3DE636',
-  'kata_exists?(id) for id that is not a 10-digit string is false' do
-    not_string = Object.new
-    refute kata_exists?(not_string)
-    nine = 'DE6369A32'
-    assert_equal 9, nine.length
-    refute kata_exists?(nine)
-  end
-
-  test '9D3DB6ED',
-  'kata.exists?(id) for 10 digit id with non hex-chars is false' do
-    has_a_g = '123456789G'
-    assert_equal 10, has_a_g.length
-    refute kata_exists?(has_a_g)
-  end
-
-  test '9D3CF9F2',
-  'kata.exists?(id) for non-existing id is false' do
-    kata_id = '9D3CF9F23D'
-    assert_equal 10, kata_id.length
-    refute kata_exists?(kata_id)
-  end
-
-  test '9D3DFB05',
-  'kata.exists?(id) if kata with existing id is true' do
-    create_kata(kata_id = '9D3DFB0532')
-    assert kata_exists?(kata_id)
-  end
-
   #- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
   # completions(id)
   #- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -161,7 +127,6 @@ def storer
   test '9D381C02',
   'unstarted avatar does not exist' do
     create_kata(kata_id = '9D381C0200')
-    refute avatar_exists?(kata_id, lion)
     assert_equal [], started_avatars(kata_id)
   end
 
@@ -169,13 +134,8 @@ def storer
   'started avatars exist' do
     create_kata(kata_id = '9D316F7B00')
     assert_equal lion, start_avatar(kata_id, [lion])
-    assert avatar_exists?(kata_id, lion)
-    refute avatar_exists?(kata_id, salmon)
     assert_equal [lion], started_avatars(kata_id)
-
     assert_equal salmon, start_avatar(kata_id, [lion,salmon])
-    assert avatar_exists?(kata_id, lion)
-    assert avatar_exists?(kata_id, salmon)
     assert_equal [lion,salmon].sort, started_avatars(kata_id).sort
   end
 
@@ -268,10 +228,6 @@ def create_kata(kata_id)
     storer.create_kata(manifest)
   end
 
-  def kata_exists?(kata_id)
-    storer.kata_exists?(kata_id)
-  end
-
   def kata_manifest(kata_id)
     storer.kata_manifest(kata_id)
   end
@@ -280,10 +236,6 @@ def completed(id)
     storer.completed(id)
   end
 
-  def avatar_exists?(kata_id, avatar_name)
-    storer.avatar_exists?(kata_id, avatar_name)
-  end
-
   def start_avatar(kata_id, avatar_names)
     storer.kata_start_avatar(kata_id, avatar_names)
   end
diff --git a/test/app_lib/storer_service_test.rb b/test/app_lib/storer_service_test.rb
index b4d025546..4cb9ee5f3 100755
--- a/test/app_lib/storer_service_test.rb
+++ b/test/app_lib/storer_service_test.rb
@@ -23,13 +23,11 @@ class StorerServiceTest < AppLibTestBase
 
     assert_equal '/tmp/cyber-dojo/katas', storer.path
 
-    refute storer.kata_exists?(kata_id)
     refute all_ids.include? kata_id
 
     manifest = make_manifest(kata_id)
     storer.create_kata(manifest)
 
-    assert storer.kata_exists?(kata_id)
     assert all_ids.include? kata_id
 
     expected = manifest
@@ -39,10 +37,8 @@ class StorerServiceTest < AppLibTestBase
       assert_equal value, actual[key.to_s]
     end
     assert_equal kata_id, storer.completed(kata_id[0..5])
-    refute storer.avatar_exists?(kata_id, lion)
     assert_equal [], storer.kata_started_avatars(kata_id)
     assert_equal lion, storer.kata_start_avatar(kata_id, [lion])
-    assert storer.avatar_exists?(kata_id, lion)
     assert_equal [lion], storer.kata_started_avatars(kata_id)
     files0 = storer.kata_manifest(kata_id)['visible_files']
     assert_equal files0, storer.tag_visible_files(kata_id, lion, tag=0)
diff --git a/test/app_models/katas_test.rb b/test/app_models/katas_test.rb
index 2f87931ce..6323e56b2 100755
--- a/test/app_models/katas_test.rb
+++ b/test/app_models/katas_test.rb
@@ -12,7 +12,7 @@ def setup
   #- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 
   test 'F3B8B1',
-  'katas[bad-id] is nil' do
+  'katas[bad-id] is not nil but any access to storer service raises' do
     bad_ids = [
       nil,          # not string
       Object.new,   # not string
@@ -22,11 +22,9 @@ def setup
       '123456789S'  # not 0-9A-F
     ]
     bad_ids.each do |bad_id|
-      begin
-        kata = katas[bad_id]
-        assert_nil kata
-      rescue StandardError
-      end
+      kata = katas[bad_id]
+      refute_nil kata
+      assert_raises { kata.age }
     end
   end
 
diff --git a/test/print_coverage_summary.rb b/test/print_coverage_summary.rb
index f9c3879c7..15e914163 100644
--- a/test/print_coverage_summary.rb
+++ b/test/print_coverage_summary.rb
@@ -179,7 +179,7 @@ def gather_done(stats, totals)
      [ 'total assertions per sec > 50', totals[:assertions_per_sec] > 50 ]
   ]
   done << coverage(stats, 'app_helpers')         if modules.include? 'app_helpers'
-  done << coverage(stats, 'app_lib')             if modules.include? 'app_lib'
+  done << coverage(stats, 'app_lib', 99)         if modules.include? 'app_lib'
   done << coverage(stats, 'app_models')          if modules.include? 'app_models'
   done << coverage(stats, 'lib')                 if modules.include? 'lib'
   done << coverage(stats, 'app_controllers', 98) if modules.include? 'app_controllers'