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

Introduce sclang (interpreter) lexer, parser, and compiler regression tests #2751

Merged
merged 180 commits into from
May 6, 2017
Merged

Introduce sclang (interpreter) lexer, parser, and compiler regression tests #2751

merged 180 commits into from
May 6, 2017

Conversation

mossheim
Copy link
Contributor

@mossheim mossheim commented Feb 27, 2017

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 interpretations or bytecodeifications 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:

hello_aaa_world
hello_aab_world
hello_aac_world
...
hello_mqx_world
...
hello_zzy_world
hello_zzz_world

Generally, two methods are used:

  • Compile the string. If that succeeds, interpret it. Record the resulting object using .asString, and if applicable, the resulting object's class.
  • Compile the string. If that succeeds, record its FunctionDef's bytecodes.

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

  • Lexer:
    • strings up to length 3
    • prefix/suffix combos for line and block comments, { }, code bits added before and after
  • Parser:
    • strings up to length 8 with an alphabet consisting of parser-important things like "var", "arg", ";"
    • a prefix/suffix combo for { }

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

  • Thorough review of .sc files
  • Is this the right folder? I don't want to run these tests every time in Travis; they take almost an hour, and rightly so. They should really only apply when deeper layers of sclang have been modified.
  • Validate across platforms (unlikely to conflict) (I was so naïve)
  • Validate across locales (possible conflicts)
  • Add tests targeting specific subroutines of the lexer (accidentals, floats, radix/hex notation) - that's why this is a work-in-progress
  • Add tests that target the compiler by repurposing the lexer tests
  • Finish documentation via readme
  • Add notes elsewhere about the existence of these tests

Brian Heim added 30 commits February 26, 2017 17:43
- 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
@nhthn
Copy link
Contributor

nhthn commented Mar 21, 2017

I already mentioned this on Gitter, but TestParser succeeds, and TestCompilerBrutal has two failures related to the presence of LID. So, all that remains to get the tests to pass are LID and the radix issue.

- 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.
@nhthn
Copy link
Contributor

nhthn commented Mar 21, 2017

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 yylex() and the Bison source file at least.

@mossheim
Copy link
Contributor Author

@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 TestLPCTestUtils as I solved each issue. It's still possible I missed something simple. In any case, you'll now get debugging messages when certain results are skipped for nan, LID, or floating-point precision reasons, which I think is a good thing to have. As the responsibilities of doOutputsMatch increase, I worry about it over-accepting results.

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.

@nhthn
Copy link
Contributor

nhthn commented Mar 22, 2017

@brianlheim

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.

You can rebase and still preserve some commit messages as desired.

LID tests are still failing for me, though:

~/git/supercollider/testsuite/sclang/lpc [sclang-tests L|✚ 1⚑ 2] 
17:32 $ tail -n +1 */*_diff
==> compiler/allChars_3_basicNoTCO_diff <==

0 entries were missing from the test file
-----------------------------------------

0 entries were missing from the validation file
-----------------------------------------------

1 entries were different (test vs validation)
---------------------------------------------
LID: "0000F2" vs "compile-error"

==> compiler/allChars_3_basicTCO_diff <==

0 entries were missing from the test file
-----------------------------------------

0 entries were missing from the validation file
-----------------------------------------------

1 entries were different (test vs validation)
---------------------------------------------
LID: "0000F2" vs "compile-error"

==> lexer/half_3_basic_diff <==

0 entries were missing from the test file
-----------------------------------------

0 entries were missing from the validation file
-----------------------------------------------

1 entries were different (test vs validation)
---------------------------------------------
LID: "LID:Meta_LID" vs "compile-error"

==> lexer/half_3_semanticPrefix_diff <==

0 entries were missing from the test file
-----------------------------------------

0 entries were missing from the validation file
-----------------------------------------------

1 entries were different (test vs validation)
---------------------------------------------
LID: "LID:Meta_LID" vs "compile-error"

==> lexer/half_3_semanticSuffix_diff <==

0 entries were missing from the test file
-----------------------------------------

0 entries were missing from the validation file
-----------------------------------------------

1 entries were different (test vs validation)
---------------------------------------------
LID: "unique:Symbol" vs "compile-error"

@mossheim
Copy link
Contributor Author

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:

[debug] LPCTestUtils: Ignoring a result because of LID class.	Input: [ L, I, D ]	Output 1: [ 4C4944, Meta_LID ]	Output 2: [ !cErr ]

I'll try to run these tests on Windows later today.

@nhthn
Copy link
Contributor

nhthn commented Mar 22, 2017

All the tests pass now! I'll do code review over the next few days, and also see what happens if I mess with yylex().

- 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.
@mossheim
Copy link
Contributor Author

These tests now also pass on Windows. (Win10+VS 2013 build)

@mossheim
Copy link
Contributor Author

LPCTestUtils has gotten pretty large (~900 loc). I could break out some of the file utilities (and corresponding tests) into a separate class...

Copy link
Contributor

@nhthn nhthn left a 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.

@mossheim
Copy link
Contributor Author

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.

@LFSaw
Copy link
Member

LFSaw commented Apr 24, 2017

((( just want to say thanks for the massive work you're investing into this... sorry for the noise amd keep it up, massively appreciated! :) )))

@mossheim
Copy link
Contributor Author

mossheim commented May 5, 2017

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!

@mossheim mossheim merged commit fc3d732 into supercollider:master May 6, 2017
@mossheim mossheim deleted the topic/sclang-tests branch May 6, 2017 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants