Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testability and build improvements #2405

Merged
merged 9 commits into from
Nov 16, 2018

Conversation

ewanmellor
Copy link
Contributor

@ewanmellor ewanmellor commented Nov 15, 2018

Miscellaneous testability and build improvements.

@ewanmellor ewanmellor force-pushed the testability-and-build-improvements branch 3 times, most recently from 42adcd8 to 500cfba Compare November 15, 2018 19:21
@parrt
Copy link
Member

parrt commented Nov 15, 2018

Does this fix previous build? Looks like that failed:

org.antlr.v4.test.runtime.swift.TestPerformance
testOne[Swift:DropLoopEntryBranchInLRRule_4](org.antlr.v4.test.runtime.swift.TestPerformance)  Time elapsed: 64.679 sec  <<< FAILURE!
junit.framework.ComparisonFailure: 
expected:<null> but was:<fatal error: unexpectedly found nil while unwrapping an Optional value
>
testOne[Swift:DropLoopEntryBranchInLRRule_3](org.antlr.v4.test.runtime.swift.TestPerformance)  Time elapsed: 7.804 sec  <<< FAILURE!
junit.framework.ComparisonFailure: 
expected:<null> but was:<fatal error: unexpectedly found nil while unwrapping an Optional value
>
...

@ewanmellor
Copy link
Contributor Author

@parrt Those test failures are fixed with my Swift 4.2 fixes, which I just filed as PR #2407. If you want to merge #2407 then I can rebase this one so that they pass (or just merge them together, they don't conflict).

@parrt
Copy link
Member

parrt commented Nov 15, 2018

@ewanmellor done! merged #2047

@ewanmellor ewanmellor force-pushed the testability-and-build-improvements branch from 500cfba to deeb74c Compare November 15, 2018 20:38
…e value.

This was always clearly a possibility, looking at the body of the method.
The newly-enabled performance tests expose this bug (and I don't know how
we've gotten away with it otherwise for so long).

The Java runtime also returns null at this point.
This means that we will see the errors if Surefire is bailing for any
reason.
This means that if you build in an unclean source tree, you don't end up with
intermediate classes in the jar.  (This hasn't broken anything that I noticed,
but it bloats the jar and I wanted to make sure it wasn't messing anything up.)

This excludes .build (the Swift runtime's build directory), target (the Java
runtime's build directory) and Swift/*.xcodeproj (which is not in the source tree
but can be created by devs who are using Xcode).
Change the unit test asserts so that we can see all the values in the
event of a failure.  When debugging an error, it's useful to see both
the stdout and stderr for the failure.  Previously we would only see
one or the other (whichever one failed the assert).

This adds a helper function BaseRuntimeTest.assertCorrectOutput.

This also removes SpecialRuntimeTestAssert, which has not been used for
2 years.
We get much more info if we let the exception rise to the top level.
Change all the subprocess calls in boot.py to check whether they succeeded,
and set the script status appropriately.

In particular, when our unit tests fail, we need the script to exit
with a failure code so that we actually notice on Travis.
Add -f netstandard1.3 to the dotnet build command used on Travis.

This has been necessary since PR antlr#2024, and the build has been silently failing
all this time.
Test scripts that ignore errors aren't very helpful.
@ewanmellor ewanmellor force-pushed the testability-and-build-improvements branch from deeb74c to 443a1d2 Compare November 15, 2018 23:33
@ewanmellor
Copy link
Contributor Author

I've rebased this on top of PR #2408, which should fix the test failures.

@parrt parrt added this to the 4.7.2 milestone Nov 16, 2018
@parrt parrt merged commit f08de81 into antlr:master Nov 16, 2018
@ewanmellor ewanmellor deleted the testability-and-build-improvements branch November 16, 2018 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants