Merge pull request #23930 from code-dot-org/staging

DTT (Staging > Test) [robo-dtt]

apps/src/maze/maze.js

@@ -39,8 +39,6 @@ module.exports = class Maze {
     this.stepSpeed = 100;
     this.animating_ = false;
 
-    this.cachedBlockStates = [];
-
     this.resultsHandler = undefined;
     this.response = undefined;
     this.result = undefined;
@@ -248,8 +246,6 @@ module.exports = class Maze {
   resetButtonClick_ = () => {
     var stepButton = document.getElementById('stepButton');
     stepButton.removeAttribute('disabled');
-
-    this.reenableCachedBlockStates_();
   };
 
   /**
@@ -493,24 +489,6 @@ module.exports = class Maze {
     if (studioApp().isUsingBlockly()) {
       // Disable toolbox while running
       Blockly.mainBlockSpaceEditor.setEnableToolbox(false);
-
-      if (stepMode) {
-        if (this.cachedBlockStates.length !== 0) {
-          throw new Error('Unexpected cachedBlockStates');
-        }
-        // Disable all blocks, caching their state first
-        Blockly.mainBlockSpace.getAllBlocks().forEach((block) => {
-          this.cachedBlockStates.push({
-            block: block,
-            movable: block.isMovable(),
-            deletable: block.isDeletable(),
-            editable: block.isEditable()
-          });
-          block.setMovable(false);
-          block.setDeletable(false);
-          block.setEditable(false);
-        });
-      }
     }
 
     this.controller.animationsController.stopIdling();
@@ -534,18 +512,6 @@ module.exports = class Maze {
     this.scheduleSingleAnimation_(0, actions, singleStep, timePerAction);
   }
 
-  reenableCachedBlockStates_() {
-    if (this.cachedBlockStates) {
-      // restore moveable/deletable/editable state from before we started stepping
-      this.cachedBlockStates.forEach(function (cached) {
-        cached.block.setMovable(cached.movable);
-        cached.block.setDeletable(cached.deletable);
-        cached.block.setEditable(cached.editable);
-      });
-      this.cachedBlockStates = [];
-    }
-  }
-
   /**
    * Called after user's code has finished being executed, giving us one more
    * chance to check if we've accomplished our goals. This is required in part
@@ -631,12 +597,11 @@ module.exports = class Maze {
   /**
    * Animates a single action
    * @param {string} action The action to animate
-   * @param {boolean} spotlightBlocks Whether or not we should highlight entire blocks
    * @param {integer} timePerStep How much time we have allocated before the next step
    */
-  animateAction_(action, spotlightBlocks, timePerStep) {
+  animateAction_(action, timePerStep) {
     if (action.blockId) {
-      studioApp().highlight(String(action.blockId), spotlightBlocks);
+      studioApp().highlight(String(action.blockId));
     }
 
     switch (action.command) {
@@ -742,7 +707,7 @@ module.exports = class Maze {
       return;
     }
 
-    this.animateAction_(actions[index], singleStep, timePerAction);
+    this.animateAction_(actions[index], timePerAction);
 
     var command = actions[index] && actions[index].command;
     var timeModifier = (this.controller.skin.actionSpeedScale && this.controller.skin.actionSpeedScale[command]) || 1;
@@ -778,7 +743,6 @@ module.exports = class Maze {
         // clicking reset.  Otherwise we can clear highlighting/disabled
         // blocks now
         if (!singleStep || this.result === ResultType.SUCCESS) {
-          this.reenableCachedBlockStates_();
           studioApp().clearHighlighting();
         }
         this.displayFeedback_();

apps/src/templates/instructions/TeacherFeedback.jsx

@@ -28,6 +28,10 @@ const styles = {
     fontStyle: 'italic',
     display: 'flex',
     alignItems: 'center'
+  },
+  footer:{
+    display: 'flex',
+    justifyContent: 'flex-end'
   }
 };
 
@@ -119,7 +123,8 @@ class TeacherFeedback extends Component {
     }
 
     const latestFeedback = this.state.latestFeedback.length > 0 ? this.state.latestFeedback[0] : null;
-    const feedbackUnchanged = (latestFeedback && this.state.comment === latestFeedback.comment);
+    const feedbackUnchanged = (latestFeedback && this.state.comment === latestFeedback.comment) ||
+      (!latestFeedback && this.state.comment.length === 0);
 
     const buttonDisabled = feedbackUnchanged || this.state.submitting || this.state.errorState === ErrorType.Load;
     const buttonText = latestFeedback ? i18n.update() : i18n.saveAndShare();

apps/src/templates/instructions/TopInstructionsCSP.jsx

@@ -292,7 +292,7 @@ class TopInstructions extends Component {
             {this.state.tabSelected === TabType.INSTRUCTIONS && ttsUrl &&
               <InlineAudio src={ttsUrl} style={audioStyle}/>
             }
-            {this.props.documentationUrl &&
+            {(this.props.documentationUrl && (this.state.tabSelected !== TabType.COMMENTS)) &&
               <PaneButton
                 iconClass="fa fa-book"
                 label={msg.documentation()}

dashboard/test/ui/features/step_mode.feature

@@ -14,17 +14,17 @@ Scenario: Step Only - Failure
   Then element "#runButton" is hidden
     And element "#resetButton" is visible
     And element "#stepButton" is not disabled
-    And block "5" has class "blocklySpotlight"
-    And block "6" doesn't have class "blocklySpotlight"
+    And block "5" has class "blocklySelected"
+    And block "6" doesn't have class "blocklySelected"
 
   # After second press, second block is highlighted and step button goes away
   When I press "stepButton"
-    And I wait until block "6" has class "blocklySpotlight"
+    And I wait until block "6" has class "blocklySelected"
   Then element "#runButton" is hidden
     And element "#resetButton" is visible
     And element "#stepButton" is disabled
-    And block "5" doesn't have class "blocklySpotlight"
-    And block "6" has class "blocklySpotlight"
+    And block "5" doesn't have class "blocklySelected"
+    And block "6" has class "blocklySelected"
 
   When I press "resetButton"
   Then element "#runButton" is hidden
@@ -83,8 +83,8 @@ Scenario: Step and Run - Stepping
   When I drag block "1" to block "5"
     And I press "stepButton"
     And I wait until "#stepButton" is enabled
-  Then block "5" has class "blocklySpotlight"
-    And block "6" doesn't have class "blocklySpotlight"
+  Then block "5" has class "blocklySelected"
+    And block "6" doesn't have class "blocklySelected"
     And element "#runButton" is hidden
     And element "#resetButton" is visible
     And element "#stepButton" is visible

dashboard/test/ui/runner.rb

@@ -577,29 +577,38 @@ def rerun_filename(test_run_string)
   "#{LOCAL_LOG_DIRECTORY}/#{test_run_string}.rerun"
 end
 
+def tag(tag, run=true)
+  return skip_tag(tag) unless run
+  " -t #{tag}"
+end
+
+def skip_tag(tag)
+  " -t 'not #{tag}'"
+end
+
 def cucumber_arguments_for_browser(browser, options)
   arguments = ' -S' # strict mode, so that we fail on undefined steps
-  arguments += ' -t ~@skip'
-  arguments += " -t #{eyes? && !browser['mobile'] ? '' : '~'}@eyes"
-  arguments += " -t #{eyes? && browser['mobile'] ? '' : '~'}@eyes_mobile"
-  arguments += ' -t ~@local_only' unless options.local
-  arguments += ' -t ~@no_mobile' if browser['mobile']
-  arguments += ' -t ~@only_mobile' unless browser['mobile']
-  arguments += ' -t ~@no_circle' if options.is_circle
-  arguments += ' -t ~@no_ie' if browser['browserName'] == 'Internet Explorer'
+  arguments += skip_tag('@skip')
+  arguments += tag('@eyes', eyes? && !browser['mobile'])
+  arguments += tag('@eyes_mobile', eyes? && browser['mobile'])
+  arguments += skip_tag('@local_only') unless options.local
+  arguments += skip_tag('@no_mobile') if browser['mobile']
+  arguments += skip_tag('@only_mobile') unless browser['mobile']
+  arguments += skip_tag('@no_circle') if options.is_circle
+  arguments += skip_tag('@no_ie') if browser['browserName'] == 'Internet Explorer'
 
   # Only run in IE during a DTT. always run locally or during circle runs.
   # Note that you may end up running in more than one browser if you use flags
   # like [test safari], [test ie] or [test firefox] during a circle run.
-  arguments += ' -t ~@only_one_browser' if browser['browserName'] != 'Internet Explorer' && !options.local && !options.is_circle
-
-  arguments += ' -t ~@chrome' if browser['browserName'] != 'chrome' && !options.local
-  arguments += ' -t ~@chrome_before_62' if browser['browserName'] != 'chrome' || browser['version'].to_i == 0 || browser['version'].to_i >= 62
-  arguments += ' -t ~@no_safari' if browser['browserName'] == 'Safari'
-  arguments += ' -t ~@no_firefox' if browser['browserName'] == 'firefox'
-  arguments += ' -t ~@webpurify' unless CDO.webpurify_key
-  arguments += ' -t ~@pegasus_db_access' unless options.pegasus_db_access
-  arguments += ' -t ~@dashboard_db_access' unless options.dashboard_db_access
+  arguments += skip_tag('@only_one_browser') if browser['browserName'] != 'Internet Explorer' && !options.local && !options.is_circle
+
+  arguments += skip_tag('@chrome') if browser['browserName'] != 'chrome' && !options.local
+  arguments += skip_tag('@chrome_before_62') if browser['browserName'] != 'chrome' || browser['version'].to_i == 0 || browser['version'].to_i >= 62
+  arguments += skip_tag('@no_safari') if browser['browserName'] == 'Safari'
+  arguments += skip_tag('@no_firefox') if browser['browserName'] == 'firefox'
+  arguments += skip_tag('@webpurify') unless CDO.webpurify_key
+  arguments += skip_tag('@pegasus_db_access') unless options.pegasus_db_access
+  arguments += skip_tag('@dashboard_db_access') unless options.dashboard_db_access
   arguments
 end
 
@@ -774,7 +783,7 @@ def run_feature(browser, feature, options)
   if scenario_count == 0 && !ENV['CI']
     skip_warning = "We didn't actually run any tests, did you mean to do this?\n".yellow
     skip_warning += <<EOS
-Check the ~excluded @tags in the cucumber command line above and in the #{feature} file:
+Check the excluded @tags in the cucumber command line above and in the #{feature} file:
   - Do the feature or scenario tags exclude #{browser_name}?
 EOS
     unless eyes?