Code cov perf and bug fix in if statements (#264)

* Added function to bast test suite to store global mocked functions

* Moved some logic to the utils

* Added logic to inject code that adds global mock checks at the start of function bodys

* Fixed a reversed check

* Removed new api in favor of expanding stubcall and updated injection code

* Updated some of the global function detection logic

* Unit tests, fixes, sample test project update

* removed console logs

* Transpile the file if we had to touch modify it

* Fixed global mocks and stubs not clearing

* updated some of the checks around transforming stubcall functions

* Made some code reusable and fixed some imports

* Added back the disablemocking logic and removed unused code

* Fixed bad ast related to noEarlyExit

* Updated bsc in tests app

* added tests for global stubcall on device

* Fixed some device tests

* Fixed more on device tests

* More test fixes as the result of moving to ast editor

* Fixed some indenting

* Fixed node test xml files being added after modifying assertions leading to crashes

* Updated the modify stub detection logic for globals

* more tests for global stub call modifications

* Fixed some race conditons and more global function detection refinments

* Removed some raw code statements for the session and fixed test formatting at a result

* removed trimLeading from mock util tests

* Removed some raw code statements in code cov and updated test file format

* Added a test showing the issue

* Fixed the code cov handling of if statements

* Updated test app for better covrage

* Fixed a bug with the reporting of missed files

* Improved code cov reporting perfomance

* perf optimization to cc reporting

* Some perf changed for code cov

* Fixed device by zero crash in reports for cov

* Fixed some issues picking the wrong scope and make sure stubcall worked with async tests

* Moved global stub clearing to clearStubs()

bsc-plugin/src/lib/rooibos/CodeCoverageProcessor.ts

@@ -1,8 +1,7 @@
 /* eslint-disable @typescript-eslint/no-var-requires */
 /* eslint-disable @typescript-eslint/no-unsafe-assignment */
 import type { BrsFile, Editor, ExpressionStatement, Program, ProgramBuilder, Statement } from 'brighterscript';
-import { Parser, isIfStatement, WalkMode, createVisitor } from 'brighterscript';
-import * as brighterscript from 'brighterscript';
+import { Parser, WalkMode, createVisitor, BinaryExpression, createToken, TokenKind, GroupingExpression, isForStatement, isBlock } from 'brighterscript';
 import type { RooibosConfig } from './RooibosConfig';
 import { RawCodeStatement } from './RawCodeStatement';
 import { BrsTranspileState } from 'brighterscript/dist/parser/BrsTranspileState';
@@ -20,23 +19,18 @@ export class CodeCoverageProcessor {
 
     private coverageBrsTemplate = `
         function RBS_CC_#ID#_reportLine(lineNumber, reportType = 1)
-            if m.global = invalid
-                '? "global is not available in this scope!! it is not possible to record coverage: #FILE_PATH#(lineNumber)"
+            _rbs_ccn = m._rbs_ccn
+            if _rbs_ccn <> invalid
+                _rbs_ccn.entry = { "f": "#ID#", "l": lineNumber, "r": reportType }
                 return true
-            else
-                if m._rbs_ccn = invalid
-                '? "Coverage maps are not created - creating now"
-                if m.global._rbs_ccn = invalid
-                    '? "Coverage maps are not created - creating now"
-                    m.global.addFields({
-                        "_rbs_ccn": createObject("roSGNode", "CodeCoverage")
-                    })
-                end if
-                m._rbs_ccn = m.global._rbs_ccn
-                end if
             end if
 
-            m._rbs_ccn.entry = {"f":"#ID#", "l":stri(lineNumber), "r":reportType}
+            _rbs_ccn = m?.global?._rbs_ccn
+            if _rbs_ccn <> invalid
+                _rbs_ccn.entry = { "f": "#ID#", "l": lineNumber, "r": reportType }
+                m._rbs_ccn = _rbs_ccn
+                return true
+            end if
             return true
         end function
     `;
@@ -65,9 +59,7 @@ export class CodeCoverageProcessor {
     private astEditor: Editor;
 
     public generateMetadata(isUsingCoverage: boolean, program: Program) {
-        if (isUsingCoverage) {
-            this.fileFactory.createCoverageComponent(program, this.expectedCoverageMap, this.filePathMap);
-        }
+        this.fileFactory.createCoverageComponent(program, this.expectedCoverageMap, this.filePathMap);
     }
 
     public addCodeCoverage(file: BrsFile, astEditor: Editor) {
@@ -89,20 +81,30 @@ export class CodeCoverageProcessor {
                 this.addStatement(ds);
                 ds.forToken.text = `${this.getFuncCallText(ds.range.start.line, CodeCoverageLineType.code)}: for`;
             },
-            IfStatement: (ds, parent, owner, key) => {
-                let ifStatement = ds;
-                while (isIfStatement(ifStatement)) {
-                    this.addStatement(ds);
-                    // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-                    (ifStatement as any).condition = new brighterscript.BinaryExpression(new RawCodeExpression(this.getFuncCallText(ds.condition.range.start.line, CodeCoverageLineType.branch)), brighterscript.createToken(brighterscript.TokenKind.And), (ifStatement as any).condition);
-                    ifStatement = ifStatement.elseBranch as any;
-                }
-                let blockStatements = (ifStatement as any)?.statements as any[] ?? [];
-                if (blockStatements?.length > 0) {
-                    // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-                    let coverageStatement = new RawCodeStatement(this.getFuncCallText((ifStatement as any).range.start.line - 1, CodeCoverageLineType.branch));
+            IfStatement: (ifStatement, parent, owner, key) => {
+                this.addStatement(ifStatement);
+                (ifStatement as any).condition = new BinaryExpression(
+                    new RawCodeExpression(this.getFuncCallText(ifStatement.condition.range.start.line, CodeCoverageLineType.condition)),
+                    createToken(TokenKind.And),
+                    new GroupingExpression({
+                        left: createToken(TokenKind.LeftParen),
+                        right: createToken(TokenKind.RightParen)
+                    }, ifStatement.condition)
+                );
+
+                let blockStatements = ifStatement?.thenBranch?.statements;
+                if (blockStatements) {
+                    let coverageStatement = new RawCodeStatement(this.getFuncCallText(ifStatement.range.start.line, CodeCoverageLineType.branch));
                     blockStatements.splice(0, 0, coverageStatement);
                 }
+
+                // Handle the else blocks
+                let elseBlock = ifStatement.elseBranch;
+                if (isBlock(elseBlock) && elseBlock.statements) {
+                    let coverageStatement = new RawCodeStatement(this.getFuncCallText(elseBlock.range.start.line - 1, CodeCoverageLineType.branch));
+                    elseBlock.statements.splice(0, 0, coverageStatement);
+                }
+
             },
             GotoStatement: (ds, parent, owner, key) => {
                 this.addStatement(ds);
@@ -143,7 +145,7 @@ export class CodeCoverageProcessor {
 
             },
             AssignmentStatement: (ds, parent, owner, key) => {
-                if (!brighterscript.isForStatement(parent)) {
+                if (!isForStatement(parent)) {
                     this.addStatement(ds);
                     this.convertStatementToCoverageStatement(ds, CodeCoverageLineType.code, owner, key);
                 }
@@ -186,6 +188,6 @@ export class CodeCoverageProcessor {
     }
 
     private getFuncCallText(lineNumber: number, lineType: CodeCoverageLineType) {
-        return `RBS_CC_${this.fileId}_reportLine(${lineNumber.toString().trim()}, ${lineType.toString().trim()})`;
+        return `RBS_CC_${this.fileId}_reportLine("${lineNumber.toString().trim()}", ${lineType.toString().trim()})`;
     }
 }

framework/src/source/CodeCoverage.brs

@@ -2,7 +2,7 @@ function init()
   m.resolvedMap = {}
   m.top.observeField("entry", "onEntryChange")
   m.top.observeField("save", "onSave")
-
+  m.results = []
 end function
 
 function setExpectedMap()
@@ -15,20 +15,24 @@ end function
 
 function onEntryChange()
   entry = m.top.entry
-  if entry <> invalid
-    lineMap = m.resolvedMap[entry.f]
-
-    if lineMap = invalid
-      lineMap = {}
-    end if
-    lineMap[entry.l] = entry.r
-
-    m.resolvedMap[entry.f] = lineMap
-  end if
+  ' defer till later
+  m.results.push(entry)
 end function
 
 function onSave()
   ? "saving data"
+  for each entry in m.results
+    if entry <> invalid
+      fileId = entry.f
+      lineMap = m.resolvedMap[fileId]
+
+      if lineMap = invalid
+        lineMap = {}
+        m.resolvedMap[fileId] = lineMap
+      end if
+      lineMap[entry.l] = entry.r
+    end if
+  end for
   m.top.resolvedMap = m.resolvedMap
   setExpectedMap()
   setFilePathMap()

framework/src/source/Coverage.bs

@@ -5,39 +5,54 @@ namespace rooibos.Coverage
       ? "There was no rooibos code coverage component - not generating coverage report"
       return
     end if
+    t = createObject("roTimespan")
     ? ""
     ? "...Generating code coverage report"
     ? ""
     m.global._rbs_ccn.save = true
     cc = m.global._rbs_ccn
+    expectedMap = cc.expectedMap
+    filePathMap = cc.filePathMap
+    resolvedMap = cc.resolvedMap
+
     hitFiles = []
     missFiles = []
     allLinesCount = 0
     allLinesHit = 0
-    for each key in cc.expectedMap
-      filename = cc.filePathMap[key]
-      expectedCount = cc.expectedMap[key].count()
+    for each key in expectedMap
+      filename = filePathMap[key]
+      expectedCount = expectedMap[key].count()
       allLinesCount += expectedCount
       if expectedCount > 0
-        if cc.resolvedMap[key] <> invalid
-          resolvedCount = cc.resolvedMap[key].count()
+        if resolvedMap[key] <> invalid
+          resolvedCount = resolvedMap[key].count()
           allLinesHit += resolvedCount
-          resolvedPercent = (resolvedCount / expectedCount) * 100
+          if resolvedCount = 0
+            resolvedPercent = 0
+          else
+            resolvedPercent = (resolvedCount / expectedCount) * 100
+          end if
           hitFiles.push({ percent: resolvedPercent, text: filename + ": " + str(resolvedPercent).trim() + "% (" + stri(resolvedCount).trim() + "/" + stri(expectedCount).trim() + ")" })
         else
           missFiles.push(filename + ": MISS!")
         end if
+      else
+        missFiles.push(filename + ": MISS!")
       end if
     end for
-    allLinesPercent = (allLinesHit / allLinesCount) * 100
+    if allLinesHit = 0
+      allLinesPercent = 0
+    else
+      allLinesPercent = (allLinesHit / allLinesCount) * 100
+    end if
     ? ""
     ? ""
     ? "+++++++++++++++++++++++++++++++++++++++++++"
     ? "Code Coverage Report"
     ? "+++++++++++++++++++++++++++++++++++++++++++"
     ? ""
     ? "Total Coverage: " ; str(allLinesPercent).trim() ; "% (" ; stri(allLinesHit).trim() ; "/" + stri(allLinesCount).trim() ; ")"
-    ? "Files: " ; cc.resolvedMap.count(); "/" ; cc.expectedMap.count()
+    ? "Files: " ; resolvedMap.count(); "/" ; expectedMap.count()
     ? ""
     ? "HIT FILES"
     ? "---------"
@@ -51,7 +66,10 @@ namespace rooibos.Coverage
     for i = 0 to missFiles.count() - 1
       ? missFiles[i]
     end for
-
+    ? ""
+    ? "+++++++++++++++++++++++++++++++++++++++++++"
+    ? "Code Coverage Report Complete"; t.totalMilliseconds(); "ms"
+    ? "+++++++++++++++++++++++++++++++++++++++++++"
   end function
 
   function createLCovOutput()

framework/src/source/Rooibos.bs

@@ -18,7 +18,10 @@ namespace rooibos
     screen.show()
 
     m.global = screen.getGlobalNode()
-    m.global.addFields({ "testsScene": scene })
+    m.global.addFields({
+      "testsScene": scene
+      "_rbs_ccn": createObject("roSGNode", "CodeCoverage")' bs:disable-line
+    })
 
     if scene.hasField("isReadyToStartTests") and scene.isReadyToStartTests = false
       ? "The scene is not ready yet - waiting for it to set isReadyToStartTests to true"

tests/src/components/NodeExample.bs

@@ -16,27 +16,6 @@ function SetLabelText(newText = "") as void
     m.nameText.text = newText
 end function
 
-
-function r_real_Init() as void
-  if m.top.stubs["init"] <> invalid
-    'mock call here
-  else
-    Init()
-  end if
-end function
-
-function r_real_HelloFromNode(name, age) as string
-  return "HELLO " + name + " age:" + stri(age)
-end function
-
-function r_real_UpdateState(newState) as void
-  m.top.state = newState
-end function
-
-function r_real_SetLabelText(newText) as void
-  m.nameText.text = newText
-end function
-
 function delayCall(delay, callback) as void
     timer = createObject("roSgNode", "Timer")
     timer.update({

tests/src/components/NodeExample.xml

@@ -2,4 +2,10 @@
 <component
     name="NodeExample"
     extends="Group">
+    <interface>
+        <field id="state" type="string" />
+    </interface>
+    <children>
+        <Label id="nameText" />
+    </children>
 </component>

tests/src/source/Main.bs

@@ -1,8 +1,5 @@
 
 function Main(args)
-
-  ? "here is my code"
-  ? "hello"
 end function
 
 function globalFunctionWithReturn() as dynamic

tests/src/source/NewExpectSyntax.spec.bs

@@ -319,6 +319,15 @@ namespace tests
 
     @it("stubs namespace and then cleans it")
     function _()
+      getGlobalAA().wasCalled = false
+      m.stubCall(testNamespace.functionWithReturn, function()
+        m.wasCalled = true
+        return true
+      end function)
+
+      m.assertTrue(testNamespace.functionWithReturn())
+      m.assertTrue(getGlobalAA().wasCalled)
+
       getGlobalAA().wasCalled = false
       m.stubCall(testNamespace.functionWithoutReturn, sub()
         m.wasCalled = true
@@ -329,6 +338,10 @@ namespace tests
 
       m.cleanStubs()
 
+      m.assertFalse(testNamespace.functionWithReturn())
+      m.assertFalse(getGlobalAA().wasCalled)
+
+      getGlobalAA().wasCalled = true
       m.assertInvalid(testNamespace.functionWithoutReturn())
       m.assertFalse(getGlobalAA().wasCalled)
 
@@ -337,6 +350,15 @@ namespace tests
 
     @it("stubs global and then cleans it")
     function _()
+      getGlobalAA().wasCalled = false
+      m.stubCall(globalFunctionWithReturn, function()
+        m.wasCalled = true
+        return true
+      end function)
+
+      m.assertTrue(globalFunctionWithReturn())
+      m.assertTrue(getGlobalAA().wasCalled)
+
       getGlobalAA().wasCalled = false
       m.stubCall(globalFunctionWithoutReturn, sub()
         m.wasCalled = true
@@ -347,6 +369,10 @@ namespace tests
 
       m.cleanStubs()
 
+      m.assertFalse(globalFunctionWithReturn())
+      m.assertFalse(getGlobalAA().wasCalled)
+
+      getGlobalAA().wasCalled = true
       m.assertInvalid(globalFunctionWithoutReturn())
       m.assertFalse(getGlobalAA().wasCalled)
 

tests/src/source/NodeExample.spec.bs

@@ -68,6 +68,29 @@ namespace tests
       m.timer.observeFieldScoped("fire", callback)
       m.timer.control = "start"
     end function
+
+    @it("updates state")
+    @params("start")
+    @params("stop")
+    @params("error")
+    function _(state)
+      m.node.top.state = ""
+      UpdateState(state)
+      m.assertEqual(m.node.top.state, state)
+    end function
+
+    @it("updates name text")
+    @params("jon")
+    @params("ringo")
+    @params("ringo")
+    @params("ringo")
+    @params("george")
+    @params("paul")
+    function _(text)
+      m.node.nameText.text = ""
+      SetLabelText(text)
+      m.assertEqual(m.node.nameText.text, text)
+    end function
   end class
 end namespace