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

Factor out CompilerAssert, TestFramework, ILChecker and Utilities #9350

Merged
merged 10 commits into from
Jun 1, 2020

Conversation

vzarytovskii
Copy link
Member

@vzarytovskii vzarytovskii commented May 31, 2020

As part of the tests refactoring and restructuring initiaitive, created a new project with test helpers.
As a first step - moved CompilerAssert, TestFramework, ILChecker and Utilities there, so it will be easier to reuse in the future.

PS: Naming is hard, I've called it FSharp.TestHelpers for now, can be renamed if necessary.

@vzarytovskii vzarytovskii requested review from TIHan and KevinRansom May 31, 2020 21:24
@vzarytovskii vzarytovskii self-assigned this May 31, 2020
@KevinRansom KevinRansom requested a review from brettfo May 31, 2020 22:58
@KevinRansom
Copy link
Member

Nice start.

Copy link
Member

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Looks great.

tests/FSharp.TestHelpers/Utilities.fs Show resolved Hide resolved
@vzarytovskii vzarytovskii marked this pull request as ready for review June 1, 2020 13:29
Copy link
Contributor

@TIHan TIHan left a comment

Choose a reason for hiding this comment

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

This looks good. Since we don't know what the name is yet, the current name is fine and can address it later.

Glad we are starting to separate this.

@KevinRansom
Copy link
Member

@vzarytovskii , merge this when you ever want.

@vzarytovskii vzarytovskii merged commit aaeb058 into dotnet:master Jun 1, 2020
@@ -31,13 +31,16 @@ let FSC_BASIC = FSC_OPT_PLUS_DEBUG
let FSI_BASIC = FSI_FILE
#endif

let inline getTestsDirectory dir = __SOURCE_DIRECTORY__ ++ dir
let testConfig' = getTestsDirectory >> testConfig
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel it would be better to hide the other testConfig rather than making a "prime":

  • minimizes the changes
  • the underlying testConfig is not used anywhere

I stumbled on this change while rebasing a PR where I add a test in this file, it would have just worked using the F# feature of hidding symbols.

Copy link
Member Author

Choose a reason for hiding this comment

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

It makes sense, will most likely be done as part of bigger tests refactoring.

nosami pushed a commit to xamarin/visualfsharp that referenced this pull request Feb 23, 2021
…tnet#9350)

* [WIP] Factor out CompilerAsserts, TestFramework, ILChecker to separate library

* [WIP] +NUnit reference; Put Utilities.fs under module

* Fixed tests to use new helpers library

* Added <NoWarn>3186</NoWarn> to the FSharpSuite.Tests.

* Added missing import for the TestHelpers.

* Added missing solution entries for the project

* Fix tests.fs to run tests with proper path

* Fixed more paths in the tests

* More fixes to test paths

* More fixes to test paths (again)
Sign up for free to 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.

7 participants