Skip to content

Commit

Permalink
fixes for #215
Browse files Browse the repository at this point in the history
  • Loading branch information
@lefthandedgoat
lefthandedgoat committed Oct 5, 2015
1 parent 1914ea2 commit a5705ca
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
6 changes: 5 additions & 1 deletion src/canopy/configuration.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,8 @@ let mutable configuredFinders = finders.defaultFinders
let mutable writeToSelectWithOptionValue = true
let mutable optimizeBySkippingIFrameCheck = false
let mutable optimizeByDisablingCoverageReport = false
let mutable showInfoDiv = true
let mutable showInfoDiv = true
let mutable failureScreenshotsEnabled = true
let mutable skipAllTestsOnFailure = false
let mutable skipRemainingTestsInContextOnFailure = false
let mutable skipNextTest = false
10 changes: 8 additions & 2 deletions src/canopy/runner.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,16 @@ let pass () =
reporter.pass ()

let fail (ex : Exception) id =
if skipAllTestsOnFailure = true || skipRemainingTestsInContextOnFailure = true then skipNextTest <- true
try
if failFast = ref true then failed <- true
failedCount <- failedCount + 1
contextFailed <- true
let f = DateTime.Now.ToString("MMM-d_HH-mm-ss-fff")
let ss = screenshot configuration.failScreenshotPath f
reporter.fail ex id ss
if failureScreenshotsEnabled = true then
let ss = screenshot configuration.failScreenshotPath f
reporter.fail ex id ss
else reporter.fail ex id Array.empty<byte>
with
| :? WebDriverException as failExc ->
//Fail during error report (likely OpenQA.Selenium.WebDriverException.WebDriverTimeoutException ).
Expand Down Expand Up @@ -95,6 +98,8 @@ let run () =
reporter.todo ()
else if System.Object.ReferenceEquals(test.Func, skipped) then
reporter.skip ()
else if skipNextTest = true then
reporter.skip ()
else
try
try
Expand Down Expand Up @@ -124,6 +129,7 @@ let run () =

suites
|> List.iter (fun s ->
if skipRemainingTestsInContextOnFailure = true && skipAllTestsOnFailure = false then skipNextTest <- false
if failed = false then
contextFailed <- false
if s.Context <> null then reporter.contextStart s.Context
Expand Down

18 comments on commit a5705ca

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn’t there be a statement to reset this flag
skipRemainingTestsInContextOnFailure <- false
at the beginning of each context?

@lefthandedgoat
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line

canopy/src/canopy/runner.fs

Line 132 in a5705ca

if skipRemainingTestsInContextOnFailure = true && skipAllTestsOnFailure = false then skipNextTest <- false
will reset the skipNextTest flag at the beginning of each context, it controls if a test is skipped or not.

You realistically have 3 usage scenarios
You as a user can control skipNextTest however you like for custom scenarios
Setting skipAllTestsOnFailure to true will result in all remaining (even across context) tests to be skipped after a single failure
Setting skipRemainingTestsInContextOnFailure to true will result in all remaining tests in the current context being skipped after a single failure. Following contexts will run as normal.

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you about the scenarions. But as I see it in the current code, once skipRemainingTestsInContextOnFailure is set to true by the user code, it remains as such for all subsequent contexts. I think it should be reset automatically at the beginning of each context loop.

@lefthandedgoat
Copy link
Owner Author

@lefthandedgoat lefthandedgoat commented on a5705ca Oct 6, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's true; otherwise, it will behave as a duplicate for the skipAllTestsOnFailure flag.

@lefthandedgoat
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wont duplicate skipAllTestsOnFailure because skipAllTestsOnFailure works across context

Example
with skipRemainingTestsInContextOnFailure on, the way it works in the above code

  • context 1
    • test 1 (pass)
    • test 2 (fail)
    • test 3 (skip)
    • test 4 (skip)

(skipNextTest is reset here)

  • context 2
    • test 1 (pass)
    • test 2 (pass)
    • test 3 (fail)
    • test 4 (skip)

with skipAllTestsOnFailure on

  • context 1
    • test 1 (pass)
    • test 2 (fail)
    • test 3 (skip)
    • test 4 (skip)
  • context 2
    • test 1 (skip)
    • test 2 (skip)
    • test 3 (skip)
    • test 4 (skip)

I tested this and showed a co worker and it seems reasonable to me. I can change it though since its the feature you are requesting. Just make an example that illustrates what you want if it differs.

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it works as described above, it will be perfect - that's exactly what I want. Thanks very much for your effort

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When are you going to create a new version in NuGet for this change?

@lefthandedgoat
Copy link
Owner Author

@lefthandedgoat lefthandedgoat commented on a5705ca Oct 7, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lefthandedgoat
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Available here: https://www.nuget.org/packages/canopy/0.9.35

Also updated to the latest Selenium that came out today.

Let me know if you have any issues!

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.Thank you.

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed that TeamCity reports skipped tests as passed now. Shouldn't these tests be logged as ignored or muted by the reporter?

@lefthandedgoat
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/lefthandedgoat/canopy/blob/master/src/canopy/reporters.fs#L170

Looks like it does nothing for skipped. I will have to figure out what command to send to TC to make this work.

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Chris,

Here is the documentation for TeamCity interactions.
https://confluence.jetbrains.com/display/TCD8/Build+Script+Interaction+with+TeamCity

IMO, you should define new functions for “IReporter” for skipped tests as

teamcity[testIgnored name='testName' message='ignore comment']

@galatrash
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Chris,

I've added some changes to allow reporting ignored tests in TeamCity and other reporters. Would you please review and apply these ASAP if you approve them.

diff --git a/src/canopy/reporters.fs b/src/canopy/reporters.fs
index 75da3dc..b013e39 100644
--- a/src/canopy/reporters.fs
+++ b/src/canopy/reporters.fs
@@ -6,6 +6,7 @@ open types

 type IReporter =
    abstract member testStart : string -> unit
+   abstract member testIgnore : string -> unit
    abstract member pass : unit -> unit
    abstract member fail : Exception -> string -> byte [] -> string -> unit
    abstract member todo : unit -> unit
@@ -14,7 +15,7 @@ type IReporter =
    abstract member describe : string -> unit
    abstract member contextStart : string -> unit
    abstract member contextEnd : string -> unit
-   abstract member summary : int -> int -> int -> int -> unit
+   abstract member summary : int -> int -> int -> int -> int -> unit
    abstract member write : string -> unit
    abstract member suggestSelectors : string -> string list -> unit
    abstract member quit : unit -> unit
@@ -61,7 +62,7 @@ type ConsoleReporter() =

         member this.contextEnd c = ()

-        member this.summary minutes seconds passed failed =
+        member this.summary minutes seconds passed failed ignored =
             Console.WriteLine()
             Console.WriteLine("{0} minutes {1} seconds to execute", minutes, seconds)
             if failed = 0 then
@@ -72,6 +73,10 @@ type ConsoleReporter() =
                 Console.ForegroundColor <- ConsoleColor.Red        
             Console.WriteLine("{0} failed", failed)    
             Console.ResetColor()        
+            if failed > 0 then
+                Console.ForegroundColor <- ConsoleColor.Yellow
+            Console.WriteLine("{0} ignored", ignored)
+            Console.ResetColor()

         member this.write w = Console.WriteLine w        

@@ -80,6 +85,12 @@ type ConsoleReporter() =
             Console.WriteLine("Couldn't find any elements with selector '{0}', did you mean:", selector)
             suggestions |> List.iter (fun suggestion -> Console.WriteLine("\t{0}", suggestion))
             Console.ResetColor()
+
+        member this.testIgnore id =
+            Console.ForegroundColor <- ConsoleColor.Yellow
+            Console.WriteLine("Ignored Test: {0}", id)
+            Console.ResetColor()
+
         member this.testStart id =
             Console.ForegroundColor <- ConsoleColor.DarkCyan
             Console.WriteLine("Test: {0}", id)
@@ -123,7 +134,7 @@ type TeamCityReporter() =

     interface IReporter with               
         member this.pass () = consoleReporter.pass ()
-    
+
         member this.fail ex id ss url =
             let mutable image = ""
             if not (Array.isEmpty ss) then
@@ -147,12 +158,14 @@ type TeamCityReporter() =
             teamcityReport (sprintf "testSuiteFinished name='%s'" (tcFriendlyMessage c))
             consoleReporter.contextEnd c

-        member this.summary minutes seconds passed failed = consoleReporter.summary minutes seconds passed failed        
+        member this.summary minutes seconds passed failed ignored = consoleReporter.summary minutes seconds passed failed ignored

         member this.write w = consoleReporter.write w        

         member this.suggestSelectors selector suggestions = consoleReporter.suggestSelectors selector suggestions

+        member this.testIgnore id = teamcityReport (sprintf "testIgnored name='%s'" (tcFriendlyMessage id))
+
         member this.testStart id = teamcityReport (sprintf "testStarted name='%s'" (tcFriendlyMessage id))

         member this.testEnd id = teamcityReport (sprintf "testFinished name='%s'" (tcFriendlyMessage id))
@@ -246,8 +259,7 @@ type LiveHtmlReporter(browser : BrowserStartMode, driverPath : string) =
             consoleReporter.describe d

         member this.contextStart c = 
-            contextStopWatch.Reset()
-            contextStopWatch.Start()
+            testStopWatch.Restart()
             contexts <- c :: contexts
             context <- System.Web.HttpUtility.JavaScriptStringEncode(c)
             this.swallowedJS (sprintf "addContext('%s');" context)
@@ -260,9 +272,9 @@ type LiveHtmlReporter(browser : BrowserStartMode, driverPath : string) =
             this.swallowedJS (sprintf "addTimeToContext ('%s', '%im %is');" context ellapsed.Minutes ellapsed.Seconds)
             consoleReporter.contextEnd c

-        member this.summary minutes seconds passed failed =
+        member this.summary minutes seconds passed failed ignored =
             this.swallowedJS (sprintf "setTotalTime ('%im %is');" minutes seconds)                        
-            consoleReporter.summary minutes seconds passed failed
+            consoleReporter.summary minutes seconds passed failed ignored

         member this.write w = 
             this.swallowedJS (sprintf "addMessageToTest ('%s', '%s');" context w)
@@ -271,9 +283,14 @@ type LiveHtmlReporter(browser : BrowserStartMode, driverPath : string) =
         member this.suggestSelectors selector suggestions = 
             consoleReporter.suggestSelectors selector suggestions

+        member this.testIgnore id =
+            testStopWatch.Restart()
+            test <- System.Web.HttpUtility.JavaScriptStringEncode(id)
+            this.swallowedJS (sprintf "addToContext ('%s', '%s');" context test)
+            consoleReporter.testIgnore id
+
         member this.testStart id = 
-            testStopWatch.Reset()
-            testStopWatch.Start()
+            testStopWatch.Restart()
             test <- System.Web.HttpUtility.JavaScriptStringEncode(id)
             this.swallowedJS (sprintf "addToContext ('%s', '%s');" context test)
             consoleReporter.testStart id
diff --git a/src/canopy/runner.fs b/src/canopy/runner.fs
index de34732..b1d3d50 100644
--- a/src/canopy/runner.fs
+++ b/src/canopy/runner.fs
@@ -51,6 +51,7 @@ let ( &&&&& ) description f =
     let lastSuite = incrementLastTestSuite()
     lastSuite.Always <- lastSuite.Always @ [Test(description, f, lastSuite.TotalTestsCount)]
 let mutable passedCount = 0
+let mutable ignoredCount = 0
 let mutable failedCount = 0
 let mutable contextFailed = false
 let mutable failedContexts : string list = []
@@ -60,6 +61,10 @@ let pass () =
     passedCount <- passedCount + 1
     reporter.pass ()

+let testIgnore id =
+    ignoredCount <- ignoredCount + 1
+    reporter.testIgnore id
+
 let fail (ex : Exception) id =
     if skipAllTestsOnFailure = true || skipRemainingTestsInContextOnFailure = true then skipNextTest <- true
     try
@@ -99,7 +104,7 @@ let run () =
             else if System.Object.ReferenceEquals(test.Func, skipped) then 
                 reporter.skip ()
             else if skipNextTest = true then 
-                reporter.skip ()
+                testIgnore test.Id
             else
                 try
                     try
@@ -157,7 +162,7 @@ let run () =
     history.save failedContexts

     stopWatch.Stop()    
-    reporter.summary stopWatch.Elapsed.Minutes stopWatch.Elapsed.Seconds passedCount failedCount 
+    reporter.summary stopWatch.Elapsed.Minutes stopWatch.Elapsed.Seconds passedCount failedCount ignoredCount
     reporter.suiteEnd()

 let runFor browsers =

P.S: I tried to attach this diff as a file but GITHUB kept telling me Something went really wrong, and we can’t process that file., so I pasted above instead.

@lefthandedgoat
Copy link
Owner Author

@lefthandedgoat lefthandedgoat commented on a5705ca Oct 13, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lefthandedgoat
Copy link
Owner Author

@lefthandedgoat lefthandedgoat commented on a5705ca Oct 13, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lefthandedgoat
Copy link
Owner Author

@lefthandedgoat lefthandedgoat commented on a5705ca Oct 13, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.