-
Notifications
You must be signed in to change notification settings - Fork 757
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
Introduce sclang (interpreter) lexer, parser, and compiler regression tests #2751
Conversation
- Move around var decls in testAllPossibleStrings - Initialize diffs = []
- Make constructor use a uniquely named initializer - Rename init -> initAlphabets - Convert method names from symbols to strings (can't index into symbol)
- lidString: "LID" as a hex string - strictOutputChecking: used in doOutputsMatch to turn on/off strict checking - maxline increased to 4096
I already mentioned this on Gitter, but |
- Add a simple debugging method. - Make doOutputsMatch check the input string for "LID" instead of the outputs. Also add debugging messages that alert the user when certain inputs are being considered equal that are not strictly equal. - Make doOutputsMatch consider floating point numbers that are very close equal (1e-13). Solves an issue @snappizz had on Linux. - Write tests for all changes.
I think it would be a good idea to add some comments elsewhere in the sclang source reminding future developers that these tests exist. I suggest advertising it to |
@snappizz I've added your suggestion to the main checklist at the top of this PR. I will get around to the documentation after tests pass & your code review if that's OK. Ready to test again. I added more regression tests to Also, per discussion on Gitter, this PR should be squash-merged. I'm a little sad that all my nice commit messages will disappear, but I think the code is well-commented where important or strange decisions had to be made. |
@brianlheim
You can rebase and still preserve some commit messages as desired. LID tests are still failing for me, though:
|
OK, I defined a dummy LID class on my own machine and made sure it works now. You should see a couple lines like this while running:
I'll try to run these tests on Windows later today. |
All the tests pass now! I'll do code review over the next few days, and also see what happens if I mess with |
- Use 'expected', 'actual', and 'diff' as root level directories to more clearly separate and label files. - Rewrite directory behavior for cross-platform compatibility - Add `safeMkdir` to LPCTU - Formatting changes - Add options to delete on finish and clobber existing files
- fix error causing typo - fix diff file naming - fix behavior of overwriteFiles
- Rezip and rename to reflect new directory naming system - Update readme with new information/instructions
This suffix is no longer needed with the new directory structure
Windows prints inf as '1.#INF' and nan as '1.#IND'. This commit adds code and tests for `doOutputsMatch` to handle this. Also factors some of the code out to a new function.
These tests now also pass on Windows. (Win10+VS 2013 build) |
LPCTestUtils has gotten pretty large (~900 loc). I could break out some of the file utilities (and corresponding tests) into a separate class... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extremely sorry this took so long...
some of the files mix spaces and tabs, but we can fix that later.
Wooooo. Yah I see one that I didn't intend. The rest are for alignment unless I'm missing something obvious. OK, will document and have ready for (squash) merge soon. |
((( just want to say thanks for the massive work you're investing into this... sorry for the noise amd keep it up, massively appreciated! :) ))) |
OK, done with the readme. Since this has two approved reviews and doesn't do anything with the main lib, I'm assuming this is good to merge once the CI tests pass. To reduce noise in the commit log, eliminate the snapshots that had all those text files I committed in the first stages of this project, and avoid a rebase, I'm going to merge this as a squashed commit. Thanks again everyone for your time and reviews! |
You may want to see this gist for my original proposal.
Also, this PR is apparently 1.4 mil new lines... sorry not sorry. :)I'm going with a zip archive. These tests have to be done manually anyway.
Purpose
This is a major insurance PR. I want to make some pretty significant changes to the lexer/parser to solve some longstanding issues in SC, and obviously don't want to be doing it in the dark. This provides regression tests and related utilities that will allow us to examine the behavior of the lexer/parser/compiler under a wide and thorough (in some cases, complete) set of conditions. It will also be useful for anyone else who wants to make changes to the language in the future. These tests may seem excessive, but actually they are excessive, and that's OK.
Summary of additions
The main additions are (1)
LexerParserCompilerTestUtils.sc
, which provides the bulk of test code, file formatting, and IO functionality; and (2)TestLPCTestUtils.sc
, which provides some basic functionality tests for (1). Whether or not that stays in this repository is up to you, although I like having tests (obviously).The rest of the files are either test files or validation files. The test files are short bits of code that actually run the tests, and can also be used to generate the validation files. The validation files are the real regression data that future changes will be tested against. The test utilities provide the means to clearly display the list of differences, additions, and subtractions between test and validation outputs.
Test format
Generally, the operation is formalized as a series of
interpret
ations orbytecode
ifications of "all possible strings", where each string is made of a sequence, of fixed length, of symbols from a predefined alphabet. For a given all-possible-strings test, an optional prefix and/or suffix may be provided.For instance, if the alphabet is all lowercase letters [a-z], the prefix "hello_", the suffix "_world", the sequence length 3, then the tests would cover:
Generally, two methods are used:
.asString
, and if applicable, the resulting object's class.If any step fails, a unique ID indicating that either a compile-time or run-time error occurred is recorded. In this way, we can compare the output of the parser/lexer/compiler across versions of SC to see what input is legal, what is illegal, and how certain expressions compile (or don't compile).
Summary of tests
See test files for full details, or I could add a .md file to this PR to explain what I've already explained here.
TODO
(unlikely to conflict)(I was so naïve)