Disregarding result tags in model diff (#117)

doc/src/test/java/com/mastercard/test/flow/doc/SysOutTest.java

@@ -33,7 +33,7 @@ class SysOutTest {
 				"System.err.println( \"Shutting down Chrome\" );" );
 		accept( "../report/report-core/src/test/java/com/mastercard/test/flow/report/seq/"
 				+ "Browser.java",
-				"System.err.println( \"Shutting down Chrome\" );" );
+				"System.err.println( \"Shutting down browser\" );" );
 		accept( "../report/report-core/src/test/java/com/mastercard/test/flow/report/seq/"
 				+ "AbstractSequence.java",
 				"System.out.println( operation + \" \" + Arrays.toString( args ) );",

example/app-web-ui/src/test/java/com/mastercard/test/flow/example/app/webui/WebUiTest.java

@@ -1,5 +1,9 @@
 package com.mastercard.test.flow.example.app.webui;
 
+import static com.mastercard.test.flow.util.Tags.tags;
+
+import java.util.function.Consumer;
+
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -9,6 +13,7 @@
 
 import com.mastercard.test.flow.assrt.Assertion;
 import com.mastercard.test.flow.assrt.Replay;
+import com.mastercard.test.flow.assrt.junit5.Flocessor;
 import com.mastercard.test.flow.example.app.WebUi;
 import com.mastercard.test.flow.example.app.assrt.AbstractServiceTest;
 import com.mastercard.test.flow.example.app.assrt.Browser;
@@ -53,6 +58,15 @@ public WebUiTest() {
 		super( service, Actors.WEB_UI, LOG );
 	}
 
+	@Override
+	protected Consumer<Flocessor> custom() {
+		// The flows that hit the web UI are tagged as such, so we can avoid having to
+		// build all the other flows that will never be exercised by this test
+		return flocessor -> flocessor
+				.filtering( config -> config
+						.includedTags( tags( "web" ) ) );
+	}
+
 	@Override
 	protected byte[] getResponse( Assertion assrt ) {
 		WebDriver driver = Browser.get();

report/report-core/src/test/java/com/mastercard/test/flow/report/diff/ModelDiffTest.java

@@ -2,6 +2,8 @@
 
 import static com.mastercard.test.flow.report.Mdl.Actrs.AVA;
 import static com.mastercard.test.flow.report.Mdl.Actrs.BEN;
+import static com.mastercard.test.flow.util.Tags.add;
+import static com.mastercard.test.flow.util.Tags.remove;
 import static com.mastercard.test.flow.util.Tags.set;
 
 import org.junit.jupiter.api.AfterAll;
@@ -56,9 +58,11 @@ static void setup() throws Exception {
 		Flow before = Deriver.build( removed, flow -> flow
 				.meta( data -> data
 						.description( "updated" )
-						.tags( set( "c", "d", "e" ) ) ) );
+						.tags( set( "c", "d", "e", "PASS", "FAIL" ) ) ) );
 
 		Flow after = Deriver.build( before, flow -> flow
+				.meta( data -> data
+						.tags( remove( "PASS", "FAIL" ), add( "SKIP", "ERROR" ) ) )
 				.update( i -> true, i -> {
 					i.request().set( "i", "iii" );
 					i.request().set( "e", "eee" );
@@ -120,7 +124,7 @@ void fromLink() {
 				.hasFlows(
 						"removed    [a, b, c]",
 						"unchanged  [b, c, d]",
-						"updated    [c, d, e]" );
+						"updated    [FAIL, PASS, c, d, e]" );
 	}
 
 	/**
@@ -139,7 +143,7 @@ void toLink() {
 				.hasHeader( "after change", "Test title", "13/02/2009, 23:31:30" )
 				.hasFlows(
 						"unchanged  [b, c, d]",
-						"updated    [c, d, e]",
+						"updated    [ERROR, SKIP, c, d, e]",
 						"added      [d, e, f]" );
 	}
 

report/report-core/src/test/java/com/mastercard/test/flow/report/seq/Browser.java

@@ -14,6 +14,10 @@
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.chrome.ChromeDriver;
 import org.openqa.selenium.chrome.ChromeOptions;
+import org.openqa.selenium.firefox.FirefoxBinary;
+import org.openqa.selenium.firefox.FirefoxDriver;
+import org.openqa.selenium.firefox.FirefoxOptions;
+import org.openqa.selenium.firefox.FirefoxProfile;
 
 import io.github.bonigarcia.wdm.WebDriverManager;
 
@@ -49,6 +53,11 @@ public class Browser implements
 	 */
 	private static final String SKIP_PRP = "browser.skip";
 
+	/**
+	 * The system property that controls which driver we use
+	 */
+	private static final String DRIVER_PRP = "browser.driver";
+
 	/**
 	 * Causes all tests that use the browser to be skipped
 	 */
@@ -67,26 +76,16 @@ public static WebDriver get() {
 						+ "You've asked for the browser before the shutdown hook has been set.\n"
 						+ "Only call Browser.get() in test classes with @ExtendWith(Browser.class) annotation" );
 			}
+			Driver type = Driver.CHROME;
 
-			WebDriverManager.chromedriver().setup();
-			ChromeOptions options = new ChromeOptions();
-			if( !SHOW ) {
-				options.addArguments( "--headless" );
+			for( Driver d : Driver.values() ) {
+				if( d.name().equalsIgnoreCase( System.getProperty( DRIVER_PRP ) ) ) {
+					type = d;
+					break;
+				}
 			}
-			// some oddness around browser locale changing with headful/less mode, possibly
-			// related to https://bugs.chromium.org/p/chromium/issues/detail?id=755338
-			// I'm seeing the opposite effect: headless mode used the correct locale,
-			// headful mode did not. In any case, let's set it explicitly
-			options.addArguments( "--lang=en_GB" );
-			options.addArguments( "--disable-gpu" );
-			options.addArguments( "--window-size=1400,800" );
-
-			// suppress most stdout noise. We still get a "ChromeDriver was started
-			// successfully." line for some reason
-			System.setProperty( CHROME_DRIVER_SILENT_OUTPUT_PROPERTY, "true" );
-			Logger.getLogger( "org.openqa.selenium" ).setLevel( Level.OFF );
-
-			driver = new ChromeDriver( options );
+
+			driver = type.get();
 		}
 		return driver;
 	}
@@ -134,9 +133,60 @@ private static void quitBrowser() {
 		if( driver != null ) {
 			// chromedriver seems to have an impossible-to-suppress stderr line on startup,
 			// so it's only fair that we mirror that on shutdown
-			System.err.println( "Shutting down Chrome" );
+			System.err.println( "Shutting down browser" );
 			driver.quit();
 			driver = null;
 		}
 	}
+
+	private enum Driver {
+		CHROME {
+			@Override
+			public WebDriver get() {
+				WebDriverManager.chromedriver().setup();
+				ChromeOptions options = new ChromeOptions();
+				if( !SHOW ) {
+					options.addArguments( "--headless" );
+				}
+				// some oddness around browser locale changing with headful/less mode, possibly
+				// related to https://bugs.chromium.org/p/chromium/issues/detail?id=755338
+				// I'm seeing the opposite effect: headless mode used the correct locale,
+				// headful mode did not. In any case, let's set it explicitly
+				options.addArguments( "--lang=en_GB" );
+				options.addArguments( "--disable-gpu" );
+				options.addArguments( "--window-size=1400,800" );
+
+				// suppress most stdout noise. We still get a "ChromeDriver was started
+				// successfully." line for some reason
+				System.setProperty( CHROME_DRIVER_SILENT_OUTPUT_PROPERTY, "true" );
+				Logger.getLogger( "org.openqa.selenium" ).setLevel( Level.OFF );
+
+				return new ChromeDriver( options );
+			}
+		},
+		FIREFOX {
+			@Override
+			public WebDriver get() {
+				WebDriverManager.firefoxdriver().setup();
+
+				FirefoxOptions options = new FirefoxOptions();
+
+				if( !SHOW ) {
+					FirefoxBinary binary = new FirefoxBinary();
+					binary.addCommandLineOptions( "--headless" );
+					options.setBinary( binary );
+				}
+
+				FirefoxProfile profile = new FirefoxProfile();
+				profile.setPreference( "intl.accept_languages", "en-GB" );
+				options.setProfile( profile );
+
+				options.addArguments( "--width=1400", "--height=800" );
+
+				return new FirefoxDriver( options );
+			}
+		};
+
+		public abstract WebDriver get();
+	}
 }

report/report-ng/projects/report/src/app/change-analysis/change-analysis.component.ts

@@ -4,15 +4,14 @@ import { CollatedChange, DiffPair, FlowDiffService } from '../flow-diff.service'
 import { FlowFilterService } from '../flow-filter.service';
 import { FlowPairingService } from '../flow-pairing.service';
 import { ModelDiffDataService } from '../model-diff-data.service';
-import { Entry } from '../types';
+import { Entry, removeResultTagsFrom } from '../types';
 
 @Component({
   selector: 'app-change-analysis',
   templateUrl: './change-analysis.component.html',
   styleUrls: ['./change-analysis.component.css']
 })
 export class ChangeAnalysisComponent implements OnInit {
-  private readonly resultTags: Set<string> = new Set(["PASS", "FAIL", "SKIP", "ERROR"]);
 
   changes: CollatedChange[] = [];
 
@@ -102,12 +101,14 @@ export class ChangeAnalysisComponent implements OnInit {
         }
       }
     });
-    this.resultTags.forEach(t => addedTagSet.delete(t));
-    this.resultTags.forEach(t => removedTagSet.delete(t));
-    this.resultTags.forEach(t => changedTagSet.delete(t));
-    this.resultTags.forEach(t => unchangedTagSet.delete(t));
-    this.resultTags.forEach(t => leftTags.delete(t));
-    this.resultTags.forEach(t => rightTags.delete(t));
+    removeResultTagsFrom(
+      addedTagSet,
+      removedTagSet,
+      changedTagSet,
+      unchangedTagSet,
+      leftTags,
+      rightTags
+    );
 
     let intersection = Array.from(leftTags).filter(t => rightTags.has(t));
     intersection.forEach(t => {
@@ -203,7 +204,7 @@ export class ChangeAnalysisComponent implements OnInit {
         p.right.entry.tags.forEach(t => tagSet.add(t));
       }
     });
-    this.resultTags.forEach(t => tagSet.delete(t));
+    removeResultTagsFrom(tagSet);
     let tags = Array.from(tagSet);
     tags.sort();
     return tags;

report/report-ng/projects/report/src/app/flow-diff.service.ts

@@ -2,7 +2,7 @@ import { Injectable } from '@angular/core';
 import { Diff, DIFF_EQUAL, diff_match_patch } from 'diff-match-patch';
 import { FlowPairingService } from './flow-pairing.service';
 import { ModelDiffDataService } from './model-diff-data.service';
-import { Entry, Flow, Interaction } from './types';
+import { Entry, Flow, Interaction, isResultTag } from './types';
 
 /**
  * The source of diff data: a pair of flow datas
@@ -183,7 +183,9 @@ export class FlowDiffService {
     let lines: string[] = [];
     lines.push("Identity:");
     lines.push("  " + flow.description);
-    flow.tags.forEach(t => lines.push("  " + t));
+    flow.tags
+      .filter(t => !isResultTag(t))
+      .forEach(t => lines.push("  " + t));
     lines.push("Motivation:");
     lines.push("  " + flow.motivation);
     lines.push("Context:");

report/report-ng/projects/report/src/app/flow-pairing.service.ts

@@ -1,13 +1,12 @@
 import { Injectable } from '@angular/core';
 import { ActivatedRoute } from '@angular/router';
 import { ModelDiffDataService } from './model-diff-data.service';
-import { Entry } from './types';
+import { Entry, isResultTag, removeResultTagsFrom } from './types';
 
 @Injectable({
   providedIn: 'root'
 })
 export class FlowPairingService {
-  private readonly resultTags: Set<string> = new Set(["PASS", "FAIL", "SKIP", "ERROR"]);
 
   /**
    * The pairs of flows that we can match automatically - they have the same metadata
@@ -117,7 +116,7 @@ export class FlowPairingService {
     if (match) {
       let lt: Set<string> = new Set(left.tags);
       let rt: Set<string> = new Set(right.tags);
-      this.resultTags.forEach(t => { lt.delete(t); rt.delete(t); });
+      removeResultTagsFrom(lt, rt);
       match = setEq(lt, rt);
     }
     return match;
@@ -241,8 +240,8 @@ export class FlowPairingService {
   private sortEntry(a: Entry, b: Entry): number {
     let d = 0;
     // tags form most of the identity, so sort on that first, ignoring result tags
-    let at = a.tags.filter(t => !this.resultTags.has(t));
-    let bt = b.tags.filter(t => !this.resultTags.has(t));
+    let at = a.tags.filter(t => !isResultTag(t));
+    let bt = b.tags.filter(t => !isResultTag(t));
     let idx = 0;
     while (d === 0 && idx < at.length && idx < bt.length) {
       d = at[idx].localeCompare(bt[idx]);

report/report-ng/projects/report/src/app/pair-select-item/pair-select-item.component.ts

@@ -1,5 +1,6 @@
 import { Component, Input, OnInit } from '@angular/core';
 import { DiffPair } from '../flow-diff.service';
+import { removeResultTagsFrom } from '../types';
 
 export interface ListPair { index: number, pair: DiffPair };
 
@@ -39,10 +40,7 @@ export class PairSelectItemComponent implements OnInit {
     }
 
     if (!this.showResult) {
-      dt.delete("PASS");
-      dt.delete("FAIL");
-      dt.delete("SKIP");
-      dt.delete("ERROR");
+      removeResultTagsFrom(dt);
     }
     this.tags = Array.from(dt.values());
     this.tags.sort();

report/report-ng/projects/report/src/app/types.ts

@@ -1,3 +1,25 @@
+/**
+ * The tag values that are used to denote assertion result, and so
+ * should not be considered part of the flow's identity
+ */
+const resultTags: Set<string> = new Set(["PASS", "FAIL", "SKIP", "ERROR"]);
+/**
+ * 
+ * @param tag a tag value
+ * @returns true if that value is a result tag
+ */
+export function isResultTag(tag: string) {
+  return resultTags.has(tag);
+}
+
+/**
+ * Remove result tags form the supplied sets
+ * @param tagSets Some sets of tags
+ */
+export function removeResultTagsFrom(...tagSets: Set<string>[]) {
+  tagSets.forEach(tagSet =>
+    resultTags.forEach(t => tagSet.delete(t)));
+}
 
 /**
  * Mirrors com.mastercard.test.flow.report.data.Index