Skip to content

Commit

Permalink
added stdout/stderr output validation (#147)
Browse files Browse the repository at this point in the history
Thanks to @gerdreiss for the following commits which were squashed:

- added tracking for symbol use
- adapted test case

* - using Set instead of List for used definitions

* - moved the usage state to class Definition
- moved the usage check to class Scope

* - converted Definition constructor param "used" to member variable
- added output verification to test case

* - print warning yellow and bold
- added output verification to test case

* - Stdout/stderr output validation

* - Stdout/stderr output validation

* - Stdout/stderr output validation including included and child scopes
  • Loading branch information
gerdreiss authored and hrj committed Mar 29, 2017
1 parent a260f9a commit 99dc6c2
Show file tree
Hide file tree
Showing 13 changed files with 171 additions and 9 deletions.
19 changes: 17 additions & 2 deletions base/src/main/scala/co/uproot/abandon/Ast.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package co.uproot.abandon

import java.nio.file.Paths

import scala.util.parsing.input.Position

object ASTHelper {
Expand All @@ -14,7 +16,8 @@ object ASTHelper {
}

case class InputPosition(pathOpt: Option[String], pos: Position) {
override def toString = pathOpt.getOrElse("") + " line: " + pos.line + " col: " + pos.column
def filename: String = pathOpt.map(path => Paths.get(path).normalize().getFileName.toString).getOrElse("")
override def toString: String = pathOpt.getOrElse("") + " line: " + pos.line + " col: " + pos.column
}

class InputError(msg: String) extends RuntimeException(msg)
Expand Down Expand Up @@ -273,7 +276,10 @@ sealed class ASTTangibleEntry extends ASTEntry
case class Transaction(pos: InputPosition, date: Date, posts: Seq[Post], annotationOpt: Option[String], payeeOpt: Option[String], comments: List[String]) extends ASTTangibleEntry

case class Definition(pos: InputPosition, name: String, params: List[String], rhs: Expr) extends ASTTangibleEntry {
def prettyPrint = "def %s(%s) = %s" format (name, params.mkString(", "), rhs.prettyPrint)
private var used: Boolean = false
def markAsUsed(): Unit = used = true
def isUsed: Boolean = used
def prettyPrint: String = "def %s(%s) = %s" format (name, params.mkString(", "), rhs.prettyPrint)
}

case class AccountDeclaration(name: AccountName, details: Map[String, Expr]) extends ASTTangibleEntry
Expand Down Expand Up @@ -369,4 +375,13 @@ case class Scope(entries: Seq[ASTEntry], parentOpt: Option[Scope]) extends ASTEn
includedScopes.foreach(_.checkDupes())
}
}

def checkUnusedSymbols() {
def printUnusedDefinitionWarning(d: Definition) = {
import Console.{YELLOW, BOLD, RESET}
println(s"${YELLOW}${BOLD}symbol '${d.name}' defined in ${d.pos.filename} line ${d.pos.pos.line} but never used${RESET}")
}
localDefinitions.filterNot(_.isUsed).foreach(printUnusedDefinitionWarning)
(includedScopes union childScopes).foreach(_.checkUnusedSymbols())
}
}
8 changes: 3 additions & 5 deletions base/src/main/scala/co/uproot/abandon/Eval.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package co.uproot.abandon

import com.sun.org.apache.xalan.internal.xsltc.compiler.LiteralExpr

case class Ref(name: String, argCount: Int, pos: Option[InputPosition])

object EvaluationContext {
Expand Down Expand Up @@ -51,15 +49,15 @@ class EvaluationContext(scope: Scope, localDefs: Seq[Definition]) {
}
}

def getValue(name: String, params: Seq[Expr], e: Expr) = {
def getValue(name: String, params: Seq[Expr], e: Expr): Expr = {
defined.get(name) match {
case Some(d) =>
val d = defined(name)
if (d.params.length != params.length) {
throw new InputPosError("Parameter lengths don't match for " + name, d.pos)
}
val newLocalDefs = d.params.zip(params).map(pairs => Definition(d.pos, pairs._1, Nil, pairs._2))
val result = mkContext(newLocalDefs).evaluateInternal(d.rhs)
defined(name).markAsUsed()
// println("evaluated", name, params, result)
result
case None =>
Expand Down Expand Up @@ -89,7 +87,7 @@ class EvaluationContext(scope: Scope, localDefs: Seq[Definition]) {
case UnaryNegExpr(e1) => NumericLiteralExpr(-evaluateBD(e1))(None)
case le:LiteralValue[_] => le
case IdentifierExpr(name) => getValue(name, Nil, e)
case FunctionExpr(name, arguments, _) => getValue(name, arguments.map(evaluateInternal(_)), e)
case FunctionExpr(name, arguments, _) => getValue(name, arguments.map(evaluateInternal), e)
case IfExpr(cond, e1, e2) => evaluateInternal(if(evaluateBoolean(cond)) e1 else e2)
case ConditionExpr(e1, op, e2) => {
val r1 = evaluateBD(e1)
Expand Down
1 change: 1 addition & 0 deletions base/src/main/scala/co/uproot/abandon/Process.scala
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ object Processor {
}
accState.updateAmounts(new PostGroup(detailedPosts, tx, tx.date, tx.annotationOpt, tx.payeeOpt, tx.comments))
}
scope.checkUnusedSymbols
// val accountDeclarations = filterByType[AccountDeclaration](entries)
AppState(accState)
}
Expand Down
37 changes: 35 additions & 2 deletions base/src/test/scala/co/uproot/abandon/ProcessorTest.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package co.uproot.abandon

import java.io.ByteArrayOutputStream

import org.scalatest.FlatSpec
import org.scalatest.matchers.Matcher
import org.scalatest.Matchers
import org.scalatest.Inside
import java.lang.Exception

import TestHelper._
import ParserHelper._
Expand Down Expand Up @@ -287,4 +287,37 @@ class ProcessorTest extends FlatSpec with Matchers with Inside {
}

}

it should "scope after process should contain unused definitions" in {
val testInput = """
def used1 = 1
def unused1 = 1
{
def unused2 = 2
}
2013/1/1
MyBank 14000 * used1
Income -1000
Equity -13000
"""
val parseResult = parser.abandon(scanner(testInput))
inside(parseResult) {
case parser.Success(scope: Scope, _) =>
val name = AccountName(Seq("Bank","Current"))
val alias = "MyBank"
val accounts = Seq(AccountSettings(name, Some(alias)))

val balSettings = LedgerExportSettings(None, Seq("balSheet12.txt"), showZeroAmountAccounts = false, Nil)
val settings = Settings(Nil, Nil, accounts, Nil, ReportOptions(Nil), Seq(balSettings), None, quiet = false, None, None)

val myOut = new ByteArrayOutputStream()
Console.withOut(myOut) {
Processor.process(scope, settings.accounts, None)
}

myOut.toString should (include("symbol 'unused1' defined ") and include("symbol 'unused2' defined ") and include(" but never used"))
}
}
}
18 changes: 18 additions & 0 deletions cli/src/test/scala/co/uproot/abandon/CliTest.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package co.uproot.abandon

import java.io.FileOutputStream
import java.nio.file.Paths

import fi.sn127.utils.fs.Glob
Expand Down Expand Up @@ -32,6 +33,23 @@ class CliAppTests extends DefaultArgsDirSuite {
}
}

class CliStdoutTests extends StdoutArgsDirSuite {
val basedir = Paths.get("../tests").toAbsolutePath.normalize

/**
* OK Cases with stdout/stderr output (should succeed)
*/
runDirSuiteTestCases(basedir, Glob("sclX*/**.exec")) { args: Array[String] =>
assertResult(CLIApp.SUCCEEDED) {
Console.withOut(new FileOutputStream(args(0))) {
Console.withErr(new FileOutputStream(args(1))) {
CLIApp.mainStatus(args.drop(2))
}
}
}
}
}

/**
* Test invalid input and options with CliApp
* and verify that specific exception is thrown.
Expand Down
29 changes: 29 additions & 0 deletions cli/src/test/scala/co/uproot/abandon/StdoutArgsDirSuite.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package co.uproot.abandon

import java.nio.file.Path

/**
* Abandon specific default arguments for test cases where output to stdout/stderr needs to be tested as well
*/
class StdoutArgsDirSuite extends DefaultArgsDirSuite {

/**
* Get configuration path based on name of current test case.
*
* Add stdout/stderr files to the argument array mapped by DefaultArgsDirSuite
*/
override
protected def mapArgs(testname: Path, args: Array[String]): Array[String] = {
val fu = fi.sn127.utils.fs.FileUtils(testname.getFileSystem)
val testDir = fu.getParentDirPath(testname)

val basename = fu.getBasename(testname) match { case (base, _) => base }
val stdout = "out." + basename.toString + ".stdout.txt"
val stderr = "out." + basename.toString + ".stderr.txt"

val fullPathStdout = fu.getPath(testDir.toString, stdout).toString
val fullPathStderr = fu.getPath(testDir.toString, stderr).toString

Array(fullPathStdout, fullPathStderr) ++ super.mapArgs(testname, args)
}
}
11 changes: 11 additions & 0 deletions tests/sclX0001-unused-symbols/sym01-include.ledger
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
def unusedInclude = 0
def x = 200
def y = 400

{
def nestedUnused2 = 10
}

2016/May/2
inExpenses topX
inBank
20 changes: 20 additions & 0 deletions tests/sclX0001-unused-symbols/sym01-txns.ledger
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
include "sym01-include.ledger"

def unusedTxns = 1
def topX = 2000

{
def nestedUnused1 = 10
}

2016/May/1
expenses x * 2
bank

2016/May/2
expenses y * 3
bank -y * 3

2016/May/3
expenses y - x
bank x - y
9 changes: 9 additions & 0 deletions tests/sclX0001-unused-symbols/sym01.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
inputs += sym01-txns.ledger

exports += {
type = journal
format = xml
outFiles = ["out.sym01.journal.xml"]
}

reports = []
2 changes: 2 additions & 0 deletions tests/sclX0001-unused-symbols/sym01.exec
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# format: exec
exec:
22 changes: 22 additions & 0 deletions tests/sclX0001-unused-symbols/sym01.ref.journal.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<abandon>
<journal>
<transactions>
<txn date="2016-05-01">
<post delta="400" name="expenses"></post>
<post delta="-400" name="bank"></post>
</txn>
<txn date="2016-05-02">
<post delta="1200" name="expenses"></post>
<post delta="-1200" name="bank"></post>
</txn>
<txn date="2016-05-02">
<post delta="2000" name="inExpenses"></post>
<post delta="-2000" name="inBank"></post>
</txn>
<txn date="2016-05-03">
<post delta="200" name="expenses"></post>
<post delta="-200" name="bank"></post>
</txn>
</transactions>
</journal>
</abandon>
Empty file.
4 changes: 4 additions & 0 deletions tests/sclX0001-unused-symbols/sym01.ref.stdout.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
symbol 'unusedTxns' defined in sym01-txns.ledger line 3 but never used
symbol 'unusedInclude' defined in sym01-include.ledger line 1 but never used
symbol 'nestedUnused2' defined in sym01-include.ledger line 6 but never used
symbol 'nestedUnused1' defined in sym01-txns.ledger line 7 but never used

0 comments on commit 99dc6c2

Please sign in to comment.