Skip to content

Commit

Permalink
Merge PR #317: Fix bug in AsyncTestCase
Browse files Browse the repository at this point in the history
  • Loading branch information
smarr committed Oct 31, 2022
2 parents 67eba8e + 684da02 commit 8c704bd
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 31 deletions.
5 changes: 3 additions & 2 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ kernel: ${kernel}</echo>
</target>

<!-- libgraal-jdk also implicitly builds truffle-libs -->
<target name="truffle-libs" unless="skip.truffle" depends="jvmci-libs,truffle-submodule">
<target name="truffle-libs" unless="skip.truffle" depends="truffle-submodule,jvmci-libs">
<exec executable="${mx.cmd}" dir="${compiler.dir}" failonerror="true">
<env key="DYNAMIC_IMPORTS" value="/tools" />
<env key="EXCLUDE_COMPONENTS" value="svmag,nju,nic,ni,nil,ins,dap,lsp,insight,insightheap,vvm" />
Expand All @@ -146,7 +146,7 @@ kernel: ${kernel}</echo>
</exec>
</target>

<target name="libgraal-jdk" unless="skip.truffle" depends="jvmci-libs,truffle-submodule">
<target name="libgraal-jdk" unless="skip.truffle" depends="truffle-submodule,jvmci-libs">
<exec executable="${mx.cmd}" dir="${vm.dir}" failonerror="true">
<env key="JAVA_HOME" value="${jvmci.home}" />
<!-- REM: This needs to match ./som -->
Expand Down Expand Up @@ -381,6 +381,7 @@ kernel: ${kernel}</echo>
<jvmarg value="-ea" />
<jvmarg value="-esa" />
<jvmarg value="-Dpolyglot.engine.WarnInterpreterOnly=false" />
<jvmarg value="-Dorg.slf4j.simpleLogger.defaultLogLevel=off" />
<arg line="${corelib.dir}/TestSuite/TestRunner.ns" />
</java>
</jacoco:coverage>
Expand Down
6 changes: 0 additions & 6 deletions core-lib/TestSuite/FileTests.ns
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
class FileTests usingPlatform: platform testFramework: minitest = Value (
| private TestContext = minitest TestContext.
private AsyncTestContext = minitest AsyncTestContext.
private actors = platform actors.
private Exception = platform kernel Exception.
private NotAValue = platform kernel NotAValue.
private ArgumentError = platform kernel ArgumentError.
private Vector = platform kernel Vector.
private Array = platform kernel Array.
private ObjectMirror = platform mirrors ObjectMirror.
private errorTestMessage = 'test exception 1'.

private FilePath = platform files FilePath.
private FilePattern = platform files FilePattern.
Expand Down
48 changes: 40 additions & 8 deletions core-lib/TestSuite/Minitest.ns
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,30 @@ test framework recognize this class as a test context and scan it for test
methods. Eventually, when Newspeak supports it, class metadata will be used
instead to mark classes as test contexts.

Asynchronous tests can be defined implementing a subclass of AsyncTestContext.
Both synchronous and asynchronous tests can be included. Method names for synchronous tests
should begin with 'test' and for asynchronous tests with 'testAsync'.

class Math = () (
public factorial: n = (
n = 0 ifTrue: [ ^ 1 ].
^ (self <-: factorial: n - 1) <-: * n
)
)

public class MathAsyncTest = AsyncTestContext () (
public testAsyncFactorial = (
| mathP |
mathP:: (actors createActorFromValue: Math) <-: new.
^ assert: (mathP <-: factorial: 4) resolvedWith: 24.
)
) : (
TEST_CONTEXT = ()
)

This asychronous test context defines one test method, which starts with 'testAsync' and returns a promise
of the result corresponding to the evaluation of the assertion.

This important point is worth reiterating. A class is a test context if and
only if it contains a class-side method named TEST_CONTEXT. Inheriting from
TestContext by itself does NOT make a class a test context, it only provides it
Expand Down Expand Up @@ -339,9 +363,10 @@ OTHER DEALINGS IN THE SOFTWARE. *)
|)(
class TestCase env: testEnvironment selector: selector = (
(* Represents a particular test (a method with the selector that begins with
'test') in a text context. This class, unlike SUnit's class by the same
'test' or 'testAsync') in a text context. This class, unlike SUnit's class by the same
name, is internal to the framework and is not intended to be subclassed
or instantiated by framework users. See TestContext.
or instantiated by framework users. See TestContext. For asynchronous tests
see AsyncTestCase and AsyncTestContext.

An instance holds onto a test environment (which indirectly identifies
the test context class) and a selector within the class. While a test
Expand Down Expand Up @@ -419,7 +444,10 @@ OTHER DEALINGS IN THE SOFTWARE. *)
)
class AsyncTestCase env: testEnvironment selector: selector
= TestCase env: testEnvironment selector: selector ()(
= TestCase env: testEnvironment selector: selector (
(* Represents a particular test (a method with the selector that begins with
'testAsync') in a text context. See AsyncTestContext. *)
)(
public isAsync = ( ^ true )
public runUsing: tester = (
Expand All @@ -436,13 +464,13 @@ OTHER DEALINGS IN THE SOFTWARE. *)
immediately garbage-collectable, which is an important property for large test suites. *)
cleanUp.
promisePair resolve: testSuccess ]
onError: [:e |
onError: [:ex |
| exClass |
exClass:: (ClassMirror reflecting: e) classObject.
exClass:: (ClassMirror reflecting: ex) classObject.
exClass == TestFailureException ifTrue: [
promisePair resolve: (testContextInstance
ifNil: [ TestFailure case: self description: ex messageText]
ifNotNil: [:it | it createFailureResultFor: self description: ex messageText])
ifNil: [ TestFailure case: self description: ex messageText ]
ifNotNil: [:it | it createFailureResultFor: self description: ex messageText ])
] ifFalse: [
promisePair resolve: (testContextInstance
ifNil: [ TestError case: self exception: ex ]
Expand Down Expand Up @@ -758,7 +786,11 @@ OTHER DEALINGS IN THE SOFTWARE. *)
)
)
public class AsyncTestContext = TestContext <: Value ()(
public class AsyncTestContext = TestContext (
(* An object for defining and running individual asynchronous tests.
Instances of this class need to implement test methods which start with
the 'testAsync' prefix. *)
)(
assert: promise resolvedWith: expectedResolution = (
(* TODO: add timeout support *)
^ promise
Expand Down
49 changes: 42 additions & 7 deletions core-lib/TestSuite/MinitestTests.ns
Original file line number Diff line number Diff line change
@@ -1,23 +1,23 @@
class MinitestTests usingPlatform: platform testFramework: minitest = (
class MinitestTests usingPlatform: platform testFramework: minitest = Value (
(* Tests of the Minitest framework (that is, meta-tests). *)
|
private Exception = platform kernel Exception.
private testFramework = minitest.
private TestContext = minitest TestContext.
private TestCatalog = minitest TestCatalog.
private Tester = minitest Tester.
private AsyncTestContext = minitest AsyncTestContext.
private actors = platform actors.
|
) (
public class AssertionTests = TestContext (
public class AssertionTests = AsyncTestContext (
(* Describe the class in this comment. *)
| guineaPig = GuineaPigTestModule testFramework: testFramework.
catalog = TestCatalog forModule: guineaPig.
tester
|
) (
|)(
public class GuineaPigTestModule testFramework: testFramework = (
| private TestContext = testFramework TestContext. |
) (
| private TestContext = testFramework TestContext.
|)(
public class AssertEqualsTestContext = TestContext ()(
public testAssertEqualsFail = (
assert: 3 equals: 4
Expand Down Expand Up @@ -193,6 +193,24 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
TEST_CONTEXT = ()
)

public class AssertAsyncTestContext = AsyncTestContext (
(* Class for testing asynchronous assertions. *)
)(
public testAsyncAssertResolvedWithPass = (
| completionPP = actors createPromisePair. |
completionPP resolver resolve: 4.
^ assert: completionPP promise resolvedWith: 4
)

public testAsyncAssertResolvedWithFail = (
| completionPP = actors createPromisePair. |
completionPP resolver resolve: 4.
^ assert: completionPP promise resolvedWith: 5
)
): (
TEST_CONTEXT = ()
)

class GenericException = Exception (
| public messageText |
)()
Expand All @@ -212,6 +230,7 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
)

public testAsyncAssertEquals = (
| tester |
tester:: Tester testSuite: (catalog testSuiteNamed: 'AssertEqualsTestContext').
^ tester runAll whenResolved: [:r |
assert: tester errors size equals: 0.
Expand All @@ -226,6 +245,7 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
)

public testAsyncAsserts = (
| tester |
tester:: Tester testSuite: (catalog testSuiteNamed: 'AssertTestContext').
^ tester runAll whenResolved: [:r |
assert: tester errors size equals: 3.
Expand All @@ -247,6 +267,7 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
)

public testAsyncDenials = (
| tester |
tester:: Tester testSuite: (catalog testSuiteNamed: 'DenyTestContext').
^ tester runAll whenResolved: [:r |
assert: tester errors size equals: 2.
Expand All @@ -265,6 +286,7 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
)

public testAsyncResultCreation = (
| tester |
tester:: Tester testSuite: (catalog testSuiteNamed: 'ResultCreationTestContext').
^ tester runAll whenResolved: [:r |
assert: tester failures size equals: 1.
Expand All @@ -279,6 +301,7 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
)

public testAsyncShouldShouldnt = (
| tester |
tester:: Tester testSuite: (catalog testSuiteNamed: 'ShouldShouldntTestContext').
^ tester runAll whenResolved: [:r |
assert: tester errors size equals: 0.
Expand All @@ -295,6 +318,18 @@ class MinitestTests usingPlatform: platform testFramework: minitest = (
assert: tester successes containsSelector: #testShouldntIsNotFailingTestForOtherException.
]
)

public testAsyncAssertResolved = (
| tester |
tester:: Tester testSuite: (catalog testSuiteNamed: 'AssertAsyncTestContext').
^ tester runAll whenResolved: [:r |
assert: tester errors size equals: 0.
assert: tester failures size equals: 1.
assert: tester failures containsSelector: #testAsyncAssertResolvedWithFail.
assert: tester successes size equals: 1.
assert: tester successes containsSelector: #testAsyncAssertResolvedWithPass.
]
)
) : (
TEST_CONTEXT = ()
)
Expand Down
3 changes: 0 additions & 3 deletions core-lib/TestSuite/StreamTests.ns
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
class StreamTests usingPlatform: platform testFramework: minitest = Value (
| private TestContext = minitest TestContext.
private AsyncTestContext = minitest AsyncTestContext.
private actors = platform actors.
private Exception = platform kernel Exception.
private NotAValue = platform kernel NotAValue.
private Vector = platform kernel Vector.
private Array = platform kernel Array.
private ObjectMirror = platform mirrors ObjectMirror.
Expand Down
12 changes: 11 additions & 1 deletion src/som/compiler/MixinDefinition.java
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,18 @@ private boolean checkAndConfirmIsValue(final SClass superClass,
boolean declaredAsValue = superIsValue || mixinsIncludeValue || isValueClass;

if (declaredAsValue && !allSlotsAreImmutable) {
String mutableSlots = "";
for (SlotDefinition sd : slots.getValues()) {
if (!sd.isImmutable()) {
if (!mutableSlots.equals("")) {
mutableSlots += ", ";
}
mutableSlots += sd.getName().getString();
}
}

reportErrorAndExit(": The class ",
" is declared as value, but also declared mutable slots");
" is declared as value, but also declared mutable slots: " + mutableSlots);
}
if (declaredAsValue && !hasOnlyImmutableFields) {
reportErrorAndExit(": The class ",
Expand Down
5 changes: 3 additions & 2 deletions src/som/interpreter/nodes/IsValueCheckNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import som.VM;
import som.compiler.MixinDefinition.SlotDefinition;
import som.interpreter.TruffleCompiler;
import som.interpreter.Types;
import som.interpreter.nodes.IsValueCheckNodeFactory.ValueCheckNodeGen;
import som.interpreter.nodes.nary.UnaryExpressionNode;
import som.interpreter.objectstorage.ObjectLayout;
Expand Down Expand Up @@ -122,7 +123,7 @@ public Object allFieldsAreValues(final VirtualFrame frame, final SImmutableObjec
if (cachedTester.allFieldsContainValues(rcvr)) {
return rcvr;
} else {
return notAValue.signal(rcvr);
return notAValue.signal(rcvr.getSOMClass());
}
}

Expand All @@ -133,7 +134,7 @@ public Object fallback(final Object receiver) {
if (allFieldsContainValues) {
return rcvr;
}
return notAValue.signal(rcvr);
return notAValue.signal(Types.getClassOf(rcvr));
}

protected static final class FieldTester extends Node {
Expand Down
7 changes: 6 additions & 1 deletion src/som/primitives/actors/CreateActorPrim.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import bd.primitives.Primitive;
import som.VM;
import som.interpreter.Types;
import som.interpreter.actors.Actor;
import som.interpreter.actors.SFarReference;
import som.interpreter.nodes.ExceptionSignalingNode;
Expand Down Expand Up @@ -63,7 +64,11 @@ public final SFarReference createActor(final VirtualFrame frame, final Object re

@Fallback
public final Object throwNotAValueException(final Object receiver, final Object argument) {
return notAValue.signal(argument);
if (argument instanceof SClass) {
return notAValue.signal(argument);
} else {
return notAValue.signal(Types.getClassOf(argument));
}
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion src/som/primitives/processes/ChannelPrimitives.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import som.VM;
import som.compiler.AccessModifier;
import som.compiler.MixinBuilder.MixinDefinitionId;
import som.interpreter.Types;
import som.interpreter.actors.SuspendExecutionNodeGen;
import som.interpreter.nodes.ExceptionSignalingNode;
import som.interpreter.nodes.nary.BinaryComplexOperation.BinarySystemOperation;
Expand Down Expand Up @@ -316,7 +317,8 @@ public final WritePrim initialize(final VM vm) {
public final Object write(final VirtualFrame frame, final SChannelOutput out,
final Object val) {
if (!isVal.executeBoolean(frame, val)) {
notAValue.signal(val);
CompilerDirectives.transferToInterpreter();
notAValue.signal(Types.getClassOf(val));
}
try {
out.writeAndSuspendReader(val, afterRead.executeShouldHalt(), traceWrite);
Expand Down

1 comment on commit 8c704bd

@rebenchdb
Copy link

@rebenchdb rebenchdb bot commented on 8c704bd Oct 31, 2022

Choose a reason for hiding this comment

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

Performance changes for 67eba8e...8c704bd

Summary Over All Benchmarks
Summary Over All Benchmarks

Full Report

Please sign in to comment.