Skip to content

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 KevinRansom and TIHan 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
Contributor

Nice start.

Copy link
Contributor

@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.

@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
Contributor

@vzarytovskii , merge this when you ever want.

@vzarytovskii vzarytovskii merged commit aaeb058 into dotnet:master Jun 1, 2020
#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