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

Baseline parser tests #14721

Merged
merged 8 commits into from
Feb 10, 2023
Merged

Baseline parser tests #14721

merged 8 commits into from
Feb 10, 2023

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Feb 9, 2023

This is a first attempt at tackling #13484.

I migrated the existing parser tests (which asserted patterns in the AST) with baseline tests.
These tests will be more interesting to detect regressions early on.

@nojaf nojaf requested a review from a team as a code owner February 9, 2023 10:08
@nojaf nojaf requested a review from auduchinok February 9, 2023 10:08
Copy link
Member

@auduchinok auduchinok left a comment

Choose a reason for hiding this comment

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

@nojaf This is really cool!

Maybe we could move the files into subfolders, mimicking the file structure that was there before:

tests/service/data/SyntaxTree/Type/...
tests/service/data/SyntaxTree/Expression/...
tests/service/data/SyntaxTree/ModuleOrNamespace/...

We'll likely add a lot of these files, so maybe we could have some organization strategy from the start.
Otherwise, we could have this kind of structure in the file names, but I think it'll be much more difficult to maintain when there're many contributors.

tests/service/data/SyntaxTree/AbstractKeyword.fs Outdated Show resolved Hide resolved
tests/service/data/SyntaxTree/AbstractKeyword.fs.bsl Outdated Show resolved Hide resolved
Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

That's great, thanks!

__SOURCE_DIRECTORY__ usage.
@nojaf
Copy link
Contributor Author

nojaf commented Feb 9, 2023

@auduchinok I took a shortcut for __SOURCE_DIRECTORY__, I think this is ok for now.

@nojaf nojaf mentioned this pull request Feb 9, 2023
@abonie abonie merged commit 1e65a08 into dotnet:main Feb 10, 2023
Comment on lines +150 to +151
[<TestCaseSource(nameof allTestCases)>]
let ParseFile fileName =
Copy link
Member

@auduchinok auduchinok Feb 10, 2023

Choose a reason for hiding this comment

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

Is there an easy way to run/debug a specific test? It's going to be crucial when debugging some cases in the parser (e.g. debugging helped me a lot when I worked on the parser in #13352).

It would also be very handy to be able to dump tokens for a specific tests easily: it may really help when changing LexFilter, as in #13089.

@nojaf @vzarytovskii Would it be easy to have separate named tests, similar to what we have here? https://github.com/JetBrains/resharper-fsharp/blob/net231/ReSharper.FSharp/test/src/FSharp.Tests/Parsing/FSharpParserTest.fs

Copy link
Member

Choose a reason for hiding this comment

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

Ouch, I'm a bit late 😞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can debug each individual file:
image

@nojaf nojaf deleted the parsing-tests branch February 10, 2023 11:10
KevinRansom added a commit to KevinRansom/fsharp that referenced this pull request Feb 20, 2023
kant2002 pushed a commit to kant2002/fsharp that referenced this pull request Apr 1, 2023
* Add parser baseline tests.

* Remove ported tests.

* Update FSharp.Compiler.UnitTests.fsproj

* Add a newline before and after the file content.

* Move original tests into designated folder.

* Add ending newline to baseline files.

* Take __SOURCE_DIRECTORY__ result into account.

* Sanitize AST for current
__SOURCE_DIRECTORY__ usage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants