Skip to content

Instantly share code, notes, and snippets.

@krishnadey30
Last active June 17, 2019 23:11
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save krishnadey30/45dfc521b02f7b6012aa20be9eea33ce to your computer and use it in GitHub Desktop.
Save krishnadey30/45dfc521b02f7b6012aa20be9eea33ce to your computer and use it in GitHub Desktop.
This gist contains the initial draft of the UnitTest Framework - Chapel
module UnitTest {
  use Reflection;
  use TestError;

  config const skipId: int = 0;
  // This is a dummy test so that we get capture the function signature
  private
  proc testSignature(test: Test) throws { }
  var tempFcf = testSignature;
  type argType = tempFcf.type;  //Type of First Class Test Functions

  /*A test result class that can print formatted text results to a stream.*/
  class TextTestResult {
    var separator1 = "="* 70,
        separator2 = "-"* 70;
    
    proc startTest(test, indx) throws {
      stdout.write(test: string,"(",indx: string,"): ");
    }

    proc addError(test, errMsg) throws {
      stdout.writeln("ERROR");
      PrintError(errMsg);
    }

    proc addFailure(test, errMsg) throws {
      stdout.writeln("FAIL");
      PrintError(errMsg);
    }

    proc addSuccess(test) throws {
      stdout.writeln("OK");
    }

    proc addSkip(test, reason) throws {
      stdout.writeln("SKIPPED");
      PrintError(reason);
    }

    proc PrintError(err) throws {
      stdout.writeln(this.separator1);
      stdout.writeln(err);
      stdout.writeln(this.separator2);
    }
    
  }

  /*Runs the tests*/
  proc runTest(tests: argType ...?n) throws {

    // Assuming 1 global test suite for now
    // Per-module or per-class is possible too
    var testSuite = new TestSuite();
    testSuite.addTests(tests);
    var testResult = new TextTestResult();
    // if skipId == 0 then
    //   stdout.writeln("Found "+testSuite.testCount+" "+printTest(testSuite.testCount));
    for indx in (skipId+1)..testSuite.testCount {
      var test = testSuite[indx];
      try {
        // Create a test object per test
        var testObject = new Test();
        //test is a FCF:
        testResult.startTest(test: string, indx);
        test(testObject);
        testResult.addSuccess(test: string);
      }
      // A variety of catch statements will handle errors thrown
      catch e: AssertionError {
        testResult.addFailure(test:string, e:string);
        // print info of the assertion error
      }
      catch e: TestSkipped {
        testResult.addSkip(test:string, e:string);
        // Print info on test skipped
      }
      catch e: TestDependencyNotMet {
        // Pop test out of array and append to end
      }
      catch e { 
        testResult.addError(test:string, e:string);
      }
    }
    // testResult.printErrors();
    // stdout.writeln(testResult.separator2);
    // testResult.PrintResult();
  }

  class TestSuite {
    var testCount = 0;
    var _tests: [1..0] argType;
    
    proc addTest(test) lifetime this < test {
      // var tempTest = new Test();
      // param test_name = test: string;
      // if !canResolve(test_name,tempTest) then
      //   compilerError(test + " is not callable");
      this._tests.push_back(test);
      this.testCount += 1;
    }

    proc addTests(tests) lifetime this < tests {
      /*if isString(tests) then
        compilerError("tests must be an iterable, not a string");*/
      for test in tests do
        this.addTest(test);
    }

    proc this(i : int) ref: argType {
      return this._tests[i];
    }

    iter these() {
      for i in this._tests do
        yield i;
    }
  }
  
}

Example

use UnitTest;

proc test1(test: Test) throws {
  test.assertTrue(false);
}

proc test2(test: Test) throws {
  test.assertTrue(false);
}

proc test3(test: Test) throws {
  test.skip("Skipping Test 3");
}
UnitTest.runTest(test1,test2,test3);

Current Output

Run 3 test
FAILED
failures= 2
skipped= 1
@krishnadey30
Copy link
Author

krishnadey30 commented Jun 6, 2019

I have implemented all the assert functions in a record called Test.

  class Test {
    /* Unconditionally skip a test. */
    proc skip(reason: string = "") throws {
      throw new owned TestSkipped(reason);
    }

    /*
    Skip a test if the condition is true.
   */
    proc skipIf(condition: bool, reason: string = "") throws {
      if condition then
        skip(reason);
    }

    /*
      Assert that a boolean condition is true.  If it is false, prints
      'assert failed' and rasies AssertionError. 

      :arg test: the boolean condition
      :type test: `bool`
    */
    pragma "insert line file info"
    pragma "always propagate line file info"
    proc assertTrue(test: bool) throws {
      if !test then
        throw new owned AssertionError("assertTrue failed. Given expression is False");
    }

    /*
      Assert that a boolean condition is false.  If it is true, prints
      'assert failed' and raises AssertionError.

      :arg test: the boolean condition
      :type test: `bool`
    */
    pragma "insert line file info"
    pragma "always propagate line file info"
    proc assertFalse(test: bool) throws {
      if test then
        throw new owned AssertionError("assertFalse failed. Given expression is True");
    }
  }

@ben-albrecht
Copy link

Some feedback

  class AssertionError: TestError {
    proc init(details: string = "") {
      super.init(details);
    }
  }

Are these init definitions necessary?

  var tempfcf = testdummy;
  type argType = tempfcf.type;  //Type of First Class Test Functions

I think this can be just:

  type argType = testdummy.type;  //Type of First Class Test Functions

testDummy would be more consistent camelCasing, although maybe we want to call this something more specific like testSignature, since it is used to capture the function signature we expect.

        // Create a test object per test
        var testObject = new Test();
        //test is a FCF:
        test(testObject);

In future steps, we could log the test result after the test() call, and access any fields of testObject, since it was passed by reference.

  class TestSuite {
    var testCount: int;
    var _tests: LinkedList(argType);

I believe we agreed that this could be an array just as well, since arrays support push_back as well. In the near Chapel future, we'll have a List type available, which we'll switch to (think python-list, rather than linked list).

proc init() {
      this.testCount = 0;
      this.complete();
    }

this.complete() doesn't hurt to have, but note that it is implicitly added automatically. You only need it if you're calling methods from within this init. Also, you can default-initialize vtestCount above as var testCount = 0;, eliminating the need for an init definition altogether.

  iter these() ref {
      for i in _tests do
        yield i;

This ref intent will allow the caller to modify the tests as they are being looped over, e.g.

for a in array {
  a += 1;
}

Is this something we want for the array of FCFs?

Also, I tend to prefer the explicit this.{field||method} syntax as you do in your code. However, I noticed you omit it in a few areas, such as the iterator above, _tests vs. this._tests. It's up to you which style you choose, but I always encourage consistency.

High-level stylistic:

  • aim for consistent amount of newlines between functions/classes (1 is usually preferred)
  • It's a good habit to attach a chpldoc comment to all defined classes and functions, even if just a placeholder of TODO: document
  • I don't think this current version uses the Reflection module. Also, after switching to arrays, I think we can drop the use LinkedLists too.

@krishnadey30
Copy link
Author

  class AssertionError: TestError {
    proc init(details: string = "") {
      super.init(details);
    }
  }

Here init's are necessary as we may want to raise the error with custom message.


iter these() ref {
      for i in _tests do
        yield i;

This is for iterating over the tests directly like here.

for test in testSuite {
      try {
        // Create a test object per test
        var testObject = new Test();
        //test is a FCF:
        test(testObject);
      }
 }

@krishnadey30
Copy link
Author

type argType = testdummy.type;  //Type of First Class Test Functions

This is not working and Lydia Ma'am has opened an issue chapel-lang/chapel#13209.


Tried changing

var _tests: LinkedList(argType);

to

var _tests: [1..0] argType;

Got following error

./UnitTest.chpl:48: In function 'addTest':
./UnitTest.chpl:53: error: Actual argument test does not meet called function lifetime constraint
./UnitTest.chpl:48: note: consider scope of test
note: called function [domain(1,int(64),false)] borrowed chpl__fcf_type_Test_void.push_back(val: borrowed chpl__fcf_type_Test_void)
note: includes lifetime constraint this < val

@ben-albrecht
Copy link

Here init's are necessary as we may want to raise the error with custom message.

OK

This is for iterating over the tests directly like here.

Right, but won't this loop still work if we drop ref? (I could be wrong)

This is not working and Lydia Ma'am has opened an issue chapel-lang/chapel#13209.

OK, thanks!

re: LinkedList error, I think we need a lifetime statement. Here's a working solution for the reproducer you gave me:

class X {}

proc fcfSignature(test: X) {
}

proc fcfSignature2(test: X) {
}

var tempFcf = fcfSignature;
type argType = tempFcf.type;

class FcfArray {
  var list: [1..0] argType;
  proc addFCF(fcf: argType) lifetime this < fcf {
    list.push_back(fcf);
    writeln(list);
  }
}


var array = new FcfArray();
array.addFCF(fcfSignature2);

@krishnadey30
Copy link
Author

I used ref as same was mentioned in the docs.

@ben-albrecht
Copy link

I used ref as same was mentioned in the docs.

Gotcha. The ref allows you to modify the elements yielded by the iterator by reference. For example, the array these() method has a ref intent:

var array: [1..3] int;
for a in array {
  a += 1; // modifies array element by reference
}
writeln(array); // [1, 1, 1]

If it didn't have the ref, the a element would be a local copy, and array would remain unchanged:

// If array.these() didn't have a ref intent
var array: [1..3] int;
for a in array {
  a += 1; // modifies local copy of element. array remains unchanged
}
writeln(array); // [0, 0, 0]

I don't think we need to modify FCFs by reference, so I think it's safe to drop.

@krishnadey30
Copy link
Author

Okay, Thanks. I will do that.

@ben-albrecht
Copy link

I think we can remove use LinkedLists; now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment