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

Add empty line if necessary before 'open' statements #65

Closed
wants to merge 3 commits into from

Conversation

saul
Copy link
Contributor

@saul saul commented Nov 8, 2019

Couple of things I'm not sure about:

  • Open 04 - Inside module doesn't pass and I'm not sure how to convince it to use the TopLevelOpenCompletion: false setting when running the test
  • I don't know how to test completion in an empty file (see the Open 05 - Empty file.fs). The file is never really empty because of the comment at the top // ${COMPLETE_ITEM:IDisposable (in System)}. Any idea how I can get around this?

@auduchinok
Copy link
Collaborator

auduchinok commented Nov 11, 2019

Open 04 - Inside module doesn't pass and I'm not sure how to convince it to use the TopLevelOpenCompletion: false setting when running the test

It seems you have to specify the key at the member level:

[<TestSettingsKey(typeof<FSharpOptions>)>]
[<TestSettings("{ TopLevelOpenCompletion: false }")>]
[<Test>] member x.``Open 04 - Inside module``() = x.DoNamedTest()

Otherwise it fails to find the option for me:

TearDown:
  JetBrains.TestFramework.Utils.TestLoggerListener+TestWrapperException:
    Unknown option TopLevelOpenCompletion

When specified this way, it's properly passed via settings and it seems like it's findNearestPointToInsertOpenDeclaration from FCS where it chooses a wrong place to insert to. We should either fix it in FCS or not to use the API. I've had an experiment in a branch where its usages were removed so we could just disable test before that branch is merged.

I don't know how to test completion in an empty file

It can be done using TestAspectAttribute inheritors. It looks like TestSettingAttribute does exactly what you need here.

@saul
Copy link
Contributor Author

saul commented Nov 11, 2019

Hmm I find that difficult to believe. Are you sure TestSettingsKey is being used at all when specified on the member? From the examples I saw it was always on the type. Also is the setting definitely passed through? FCS definitely does the right thing as the setting can be toggled from the UI and it works correctly.

I’m not at home at the moment but I can dig into this when I’m back!

Thanks for taking the time to dig into this

auduchinok pushed a commit that referenced this pull request Dec 4, 2019
Add empty line before insert 'open' statement

Disable failing test
@auduchinok
Copy link
Collaborator

@saul Thank you for this!
Merged in 26e9b81.

@auduchinok auduchinok closed this Dec 4, 2019
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.

2 participants