Allow a developer to wait as a first Step v4. #387. Add test. (#506)

Closes #387.

Also, generally adds safety measures for when page does not exist.

Very similar to PR #459, but moving the responsibility down to a spot that most functions make use of, meaning that it'll be applied to a lot more cases. They mostly won't need it, but it might still be worth being more comprehensive.

* Allow a developer to wait as a first Step v4. #387. Add test.

Will be able to close once we've added this as an establishing
step (in addition to it being a regular step).

Also, generally adds safety measures for when page does not exist.

* Add test

Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>

Co-authored-by: Bryce Willey <Bryce.Steven.Willey@gmail.com>

CHANGELOG.md

@@ -45,6 +45,7 @@ Format:
 - Add package.json creation/overwriting to action.yml. Simplify package.json.
 - Step: `the text in the JSON variable "variable_with_text_value" should be`. Compare JSON variable with a text value to given text. See #470. Does not accept nested values, e.g. "child.name.first". Downloads all the JSON vars to a .json file in the "downloads" artifacts.
 - Step to log the page's JSON variables and values in the report. Future goal: save to file. See #454.
+- Allow a developer to wait as a first Step. See #387.
 - fix typo in the report
 
 ### Changed
@@ -54,8 +55,8 @@ Format:
 - Use alkiln v4 package.json we create during the test run in `action.yml` (as discussed in #489)
 - Remove and ignore package-lock.json so that our tests will behave more like our users' tests
 - Use API key to access da server, create projects, pull code, delete projects, and check for server restart.
-- Add warning in steps.js for name input with too many parts.
-- Create test in reports features to test the warning.
+- Add warning in steps.js for name input with too many parts. Add test in reports.
+- fix typo in the report
 
 <!-- ## [3.0.1-peer-deps.1] - 2021-12-07 -->
 ### Removed

lib/scope.js

@@ -2,9 +2,11 @@ const { expect } = require('chai');
 const cheerio = require('cheerio');
 const path = require('path');
 const fs = require('fs');
-
 const safe_filename = require("sanitize-filename");
+
+// Ours
 const env_vars = require('./utils/env_vars');
+const time = require('./utils/time');
 
 
 let ordinal_to_integer = {
@@ -215,37 +217,39 @@ module.exports = {
     *    TODO: discuss splitting into `afterField` and `afterStep` or something. Or just change name
     *    TODO: Update to latest cucumberjs, which as `AfterStep` */
 
-    // Use a while loop until the below does not error with `Error: Execution context was destroyed`
-    // Temporary fix for race condition. See https://github.com/plocket/docassemble-cucumber/issues/190
-    let error_id_elem;
-    while ( true ) {
-      try {
-        // Waiting for `page` to exist. '#da-retry' does not need to exist, it's incidental at this point.
-        // If this doesn't throw an error, then the page exists and the result is valid.
-        error_id_elem = await scope.page.$('#da-retry');
-        break;
-      } catch ( err ) {}
-    }
+    if ( scope.page ) {
+      // Use a while loop until the below does not error with `Error: Execution context was destroyed`
+      // Temporary fix for race condition. See https://github.com/plocket/docassemble-cucumber/issues/190
+      let error_id_elem;
+      while ( true ) {
+        try {
+          // Waiting for `page` to exist. '#da-retry' does not need to exist, it's incidental at this point.
+          // If this doesn't throw an error, then the page exists and the result is valid.
+          error_id_elem = await scope.page.$('#da-retry');
+          break;
+        } catch ( err ) {}
+      }
 
-    // If there's a system error, throw it
-    if ( error_id_elem ) {
-      let error_elem = await scope.page.$('blockquote')
-      let error_text_prop = await error_elem.getProperty( 'textContent' );
-      let system_error_text = await error_text_prop.jsonValue();
+      // If there's a system error, throw it
+      if ( error_id_elem ) {
+        let error_elem = await scope.page.$('blockquote')
+        let error_text_prop = await error_elem.getProperty( 'textContent' );
+        let system_error_text = await error_text_prop.jsonValue();
 
-      await scope.addToReport(scope, { type: `error`, value: system_error_text });
-      throw system_error_text;
-    };
+        await scope.addToReport(scope, { type: `error`, value: system_error_text });
+        throw system_error_text;
+      };
 
-    // RESET
+      // Wait for elements that might get revealed
+      if ( options.waitForShowIf ) { await scope.waitForShowIf( scope ); }
+    }  // ends if scope.page
+
+    // Reset navigation (TODO: discuss use for observational steps)
     if ( options.navigated ) { scope.navigated = true; }  // Consider `daPageLoad` event
     else { scope.navigated = false; }
-
-    // WAITING
-    if ( options.waitForShowIf ) { await scope.waitForShowIf(scope); }
-    // Plain old waiting. Yeah, I abstracted it into here so only
-    // one thing was being called at the end
-    if ( options.waitForTimeout ) { await scope.page.waitForTimeout( options.waitForTimeout ); }
+
+    // Plain old waiting. Here to avoid doing this in each individual place
+    if ( options.waitForTimeout ) { await time.waitForTimeout( options.waitForTimeout ); }
   },  // Ends afterStep
 
   tapElement: async function tapElement(scope, elem) {

lib/steps.js

@@ -17,6 +17,7 @@ const { expect } = require('chai');
 const puppeteer = require('puppeteer');
 const { v4: uuidv4 } = require('uuid');
 
+// Ours
 const scope = require('./scope');
 const env_vars = require('./utils/env_vars');
 
@@ -848,15 +849,18 @@ After(async function(scenario) {
 
   if (scenario.result.status !== `PASSED`) {
     if ( env_vars.DEBUG ) {
-      // When debugging locally, pause to allow dev to examine the page
       console.log( `Non-passed status occurred while running test:`, scenario.result.status );
-      await scope.page.waitForTimeout( 60 * 1000 );
+      // If there's a page visible, give us time to examine it
+      if ( scope.page ) { await scope.page.waitForTimeout( 60 * 1000 ); }
     }
-    // Save/download a picture of the screen during the unexpected status
-    let safe_filename = await scope.getSafeScenarioFilename( scope, { prefix: `error` });
-    await scope.page.screenshot({ path: `${ safe_filename }.jpg`, type: `jpeg`, fullPage: true });
 
-  }
+    await scope.addToReport(scope, { type: `outcome`, value: `**-- Scenario Failed --**` });
+    if ( scope.page ) {
+      // Save/download a picture of the screen that's showing during the unexpected status
+      let safe_filename = await scope.getSafeScenarioFilename( scope, { prefix: `error` });
+      await scope.page.screenshot({ path: `${ safe_filename }.jpg`, type: `jpeg`, fullPage: true });
+    }
+  }  // Ends if not passed
 
   if (scenario.result.status === `FAILED`) {
     await scope.addToReport(scope, { type: `outcome`, value: `**-- Scenario Failed --**` });
@@ -907,7 +911,7 @@ After(async function(scenario) {
 
     await scope.page.close();
     scope.page = null;
-  }
+  }  // ends if scope.page
 });
 
 AfterAll(async function() {

tests/features/establishing_steps.feature

@@ -20,3 +20,13 @@ Scenario: I am able to set a custom wait time before an interview has been loade
 Scenario: Interview name includes url args
   Given I start the interview at "url_args.yml&from=theinternets&random=zoo"
   Then I should see the phrase "zoo"
+
+@slow @e4 @wait_first
+Scenario: I can wait as a first step in a test
+  Given I wait 1 second
+
+@slow @e5 @wait_first
+Scenario: Interview name includes url args with a wait
+  Given I wait 1 second
+  Then I start the interview at "url_args.yml&from=theinternets&random=zoo"
+  Then I should see the phrase "zoo"