Skip to content

Simplified compiler test framework - #9721

Merged
vzarytovskii merged 21 commits into
dotnet:masterfrom
vzarytovskii:simplified-compiler-test-framework
Jul 23, 2020
Merged

Simplified compiler test framework#9721
vzarytovskii merged 21 commits into
dotnet:masterfrom
vzarytovskii:simplified-compiler-test-framework

Conversation

@vzarytovskii

@vzarytovskiivzarytovskii commented Jul 20, 2020

Copy link
Copy Markdown
Member

First round of implementation of #9720

There are still a bunch of stuff to implement, but first, we need to have a new base for it.

@vzarytovskiivzarytovskii changed the title [WIP] Simplified compiler test framework[WIP] [Don't review] Simplified compiler test frameworkJul 20, 2020
@abelbraaksma

abelbraaksma commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

Not really a review here, just a suggestion:

May I suggest we improve withWarning to something like withWarning (Warning 213, (Line 2, Col 1, Line 2, Col 6)), or similarly, withWarning (Warning 213, Pos.From(2, 1), Pos.To(2, 6))? I find these notoriously hard to read when working with tests (though everything here is already a biiig improvement!).

@vzarytovskii

Copy link
Copy Markdown
MemberAuthor

Not really a review here, just a suggestion:

May I suggest we improve withWarning to something like withWarning (Warning 213, (Line 2, Col 1, Line 2, Col 6)), or similarly, withWarning (Warning 213, Pos.From(2, 1), Pos.To(2, 6))? I find these notoriously hard to read when working with tests (though everything here is already a biiig improvement!).

Yeah, that's the plan - I will add a verbose versions (looks pretty much like you suggested), as well as fewer small functions which are easier to compose (like withErrorCode 123 |> onPosition ... |> withMessage ... etc), so you have freedom to how to write and how vebose you want it :)

Also - I would love to chat about the more complex testing scenarios (like one you've encountered in ExprTests).

@abelbraaksma

abelbraaksma commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

Also - I would love to chat about the more complex testing scenarios (like one you've encountered in ExprTests).

Sure, any time, you can ping me on Slack (just remember I'm on European time ;) ) and we can set up a call.

@vzarytovskii

Copy link
Copy Markdown
MemberAuthor

Also - I would love to chat about the more complex testing scenarios (like one you've encountered in ExprTests).

Sure, any time, you can ping me on Slack (just remember I'm on European time ;) ) and we can set up a call.

Yeah, I'm in Prague myself :)
Will ping you there some time during the week then.

@abelbraaksma

abelbraaksma commented Jul 20, 2020

Copy link
Copy Markdown
Contributor

Oh I love Prague! I come there at least once a year to visit friends and for a conf. I know it quite well, and of course the 🍺 🍻 🍺

@vzarytovskiivzarytovskii changed the title [WIP] [Don't review] Simplified compiler test frameworkSimplified compiler test frameworkJul 22, 2020
@vzarytovskii
vzarytovskii marked this pull request as ready for review July 22, 2020 15:36
Comment threadtests/FSharp.Compiler.ComponentTests/Interop/SimpleInteropTests.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated

// NOTE: This function will not clean up all the compiled projects after itself.
// The reason behind is so we can compose verification of test runs easier.
// TODO: We must not rely on the filesystem when compiling

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How will that be resolved?

@vzarytovskiivzarytovskiiJul 22, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I have a separate issue for tackling that.
I'm thinking on the suite teardown level we can clean up all projects/sources we've created.

@KevinRansomKevinRansom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks, great. I have suggested a few things to consider in the next update. But they shouldn't hold up merging this PR.

Nice work

Kevin

Comment threadtests/FSharp.Test.Utilities/Compiler.fs Outdated
Comment threadtests/FSharp.Test.Utilities/Compiler.fs
Warnings = []
Errors = [] }

let private typecheckFSharpSource (fsSource: FSharpCompilationSource) : CompilationResult =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it possible we would like have tests with multiple source files?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Yep, it's tracked in #9746

@vzarytovskii
vzarytovskii merged commit 3f011ec into dotnet:masterJul 23, 2020
nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
* [WIP] Simplified compiler API for testing
* [WIP] Support of references, for F#, C# projects. Added more asserts.
* Added more examples + ignoreWarnings support
* Added typecheck + baseline test helpers.
* Added running functionality
* Moved a bunch of compiler tests to a new suite
* Changed error message
* [WIP] Added CSharp compilation + referencing
* Added some simple interop tests
* [WIP] Process MetadataReference recursively
* [WIP] Added netcorapp31 support for tests
* [WIP] Termporary revert to using netcoreapp30 in the tests (instead of netcoreapp31)
* Added types to errors/warnings as oppose to just having tuples
* Fixed tests to fit newer API
* Revert libraries upgrade
* Revert change netcoreapp31 -> netcoreapp30
* Update tests/FSharp.Test.Utilities/Compiler.fs
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
* Addressed some PR comments
* Fixed missing styling inconsistensies
* Addressed feedback: recursive module instead of forward type declarations + fixed options composition.
* Changed Option.isSome -> defaultArg
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@vzarytovskii@abelbraaksma@jonsequitur@KevinRansom@cartermp