Skip to content

Send Pester describe line and info whether Pester 4.6.0 is installed to PowerShell.RunPesterTests command #856

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

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
f26755f
Revert "Fix #827 Pester TestName w/expandable str returns nothing (#8…
bergmeister Jan 20, 2019
a034574
Revert "Revert "Fix #827 Pester TestName w/expandable str returns not…
bergmeister Jan 20, 2019
c978a6f
send describe block line and info whether pester 4.6.0 is available t…
bergmeister Jan 22, 2019
725f643
send only describeBlockLineNumber as nullable int over the wire to PS…
bergmeister Jan 22, 2019
62b101b
use clearer way of passing in null when int is nullable
bergmeister Jan 23, 2019
b6b4b4c
address stylistic nitpicks by Patrick
bergmeister Jan 24, 2019
dbd42b5
Use lazy initialisation to query pester version only when it is neede…
bergmeister Jan 26, 2019
61f62f5
Merge branch 'master' of https://github.com/PowerShell/PowerShellEdit…
bergmeister Jan 29, 2019
ba32b20
use pscommand instead
bergmeister Jan 29, 2019
565f2f8
Merge branch 'master' of https://github.com/PowerShell/PowerShellEdit…
bergmeister Feb 1, 2019
32c69aa
Update src/PowerShellEditorServices.Host/CodeLens/PesterCodeLensProvi…
rjmholt Feb 8, 2019
69e7260
Merge branch 'PesterDescribeLine' of https://github.com/bergmeister/P…
bergmeister Feb 9, 2019
e5bddd0
adress style PR comments
bergmeister Feb 9, 2019
39290a2
Merge branch 'master' of https://github.com/PowerShell/PowerShellEdit…
bergmeister Feb 19, 2019
7245bd6
Use Get-Command and make calls to GetPesterCodeLens async/await to th…
bergmeister Feb 20, 2019
4ec3b3f
Oops, I did it again...
bergmeister Feb 20, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/PowerShellEditorServices.Host/CodeLens/CodeLensFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;
using Microsoft.PowerShell.EditorServices.Utility;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -99,11 +97,11 @@ private CodeLensFeature(
/// </summary>
/// <param name="scriptFile">The PowerShell script file to get CodeLenses for.</param>
/// <returns>All generated CodeLenses for the given script file.</returns>
public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile)
public async Task<CodeLens[]> ProvideCodeLenses(ScriptFile scriptFile)
{
return InvokeProviders(provider => provider.ProvideCodeLenses(scriptFile))
.SelectMany(codeLens => codeLens)
.ToArray();
var providers = await Task.WhenAll(InvokeProviders(async provider => await provider.ProvideCodeLenses(scriptFile)));
return providers.SelectMany(codeLens => codeLens)
.ToArray();
}

/// <summary>
Expand All @@ -118,7 +116,7 @@ private async Task HandleCodeLensRequestAsync(
ScriptFile scriptFile = _editorSession.Workspace.GetFile(
codeLensParams.TextDocument.Uri);

CodeLens[] codeLensResults = ProvideCodeLenses(scriptFile);
CodeLens[] codeLensResults = await ProvideCodeLenses(scriptFile);

var codeLensResponse = new LanguageServer.CodeLens[codeLensResults.Length];
for (int i = 0; i < codeLensResults.Length; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Management.Automation;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -25,6 +26,12 @@ internal class PesterCodeLensProvider : FeatureProviderBase, ICodeLensProvider
/// </summary>
private IDocumentSymbolProvider _symbolProvider;

/// <summary>
/// Pester 4.6.0 introduced a new ScriptblockFilter parameter to be able to run a test based on a line,
/// therefore knowing this information is important.
/// </summary>
private readonly Lazy<Task<bool>> _pesterV4_6_0_OrHigherAvailable;

/// <summary>
/// Create a new Pester CodeLens provider for a given editor session.
/// </summary>
Expand All @@ -33,6 +40,7 @@ public PesterCodeLensProvider(EditorSession editorSession)
{
_editorSession = editorSession;
_symbolProvider = new PesterDocumentSymbolProvider();
_pesterV4_6_0_OrHigherAvailable = new Lazy<Task<bool>>(async () => await DeterminePesterVersion());
}

/// <summary>
Expand All @@ -41,10 +49,13 @@ public PesterCodeLensProvider(EditorSession editorSession)
/// <param name="pesterSymbol">The Pester symbol to get CodeLenses for.</param>
/// <param name="scriptFile">The script file the Pester symbol comes from.</param>
/// <returns>All CodeLenses for the given Pester symbol.</returns>
private CodeLens[] GetPesterLens(
private async Task<CodeLens[]> GetPesterLens(
PesterSymbolReference pesterSymbol,
ScriptFile scriptFile)
{
// A value of null is a signal to PSES that the available Pester version does not support
// running Describe blocks by name (the test name will used instead then).
int? describeBlockLineNumber = await _pesterV4_6_0_OrHigherAvailable.Value ? (int?)pesterSymbol.ScriptRegion.StartLineNumber : null;
var codeLensResults = new CodeLens[]
{
new CodeLens(
Expand All @@ -54,7 +65,7 @@ private CodeLens[] GetPesterLens(
new ClientCommand(
"PowerShell.RunPesterTests",
"Run tests",
new object[] { scriptFile.ClientFilePath, false /* No debug */, pesterSymbol.TestName })),
new object[] { scriptFile.ClientFilePath, false /* No debug */, pesterSymbol.TestName, describeBlockLineNumber })),

new CodeLens(
this,
Expand All @@ -63,18 +74,45 @@ private CodeLens[] GetPesterLens(
new ClientCommand(
"PowerShell.RunPesterTests",
"Debug tests",
new object[] { scriptFile.ClientFilePath, true /* Run in debugger */, pesterSymbol.TestName })),
new object[] { scriptFile.ClientFilePath, true /* Run in debugger */, pesterSymbol.TestName, describeBlockLineNumber })),
};

return codeLensResults;
}

/// <summary>
/// Used to determine the value of <see cref="_pesterV4_6_0_OrHigherAvailable"/> as a background task.
/// </summary>
private async Task<bool> DeterminePesterVersion()
{
var powerShell = new PSCommand()
.AddCommand("Get-Command")
.AddParameter("Name", "Invoke-Pester");

IEnumerable<PSObject> result = await _editorSession.PowerShellContext.ExecuteCommandAsync<PSObject>(powerShell);
var pesterCommand = result.FirstOrDefault();
if (pesterCommand == null)
{
return false;
}

if (pesterCommand.BaseObject is FunctionInfo invokePesterFunction)
{
var pesterVersion = invokePesterFunction.Version;
if (pesterVersion.Major >= 4 && pesterVersion.Minor >= 6)
{
return true;
}
}
return false;
}

/// <summary>
/// Get all Pester CodeLenses for a given script file.
/// </summary>
/// <param name="scriptFile">The script file to get Pester CodeLenses for.</param>
/// <returns>All Pester CodeLenses for the given script file.</returns>
public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile)
public async Task<CodeLens[]> ProvideCodeLenses(ScriptFile scriptFile)
{
var lenses = new List<CodeLens>();
foreach (SymbolReference symbol in _symbolProvider.ProvideDocumentSymbols(scriptFile))
Expand All @@ -86,7 +124,7 @@ public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile)
continue;
}

lenses.AddRange(GetPesterLens(pesterSymbol, scriptFile));
lenses.AddRange(await GetPesterLens(pesterSymbol, scriptFile));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -50,7 +49,7 @@ public ReferencesCodeLensProvider(EditorSession editorSession)
/// </summary>
/// <param name="scriptFile">The PowerShell script file to get code lenses for.</param>
/// <returns>An array of CodeLenses describing all functions in the given script file.</returns>
public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile)
public async Task<CodeLens[]> ProvideCodeLenses(ScriptFile scriptFile)
Copy link
Member

Choose a reason for hiding this comment

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

You made this function async, but didn't add any await statements so async isn't needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to change it here as well due to the signature change in the interface. The PesterCodeLensProvider properly implements async-await, which is what counts

Copy link
Member

Choose a reason for hiding this comment

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

Interesting! Typically the compiler yells if you have an async but no await but I guess since the interface requires it, the compiler doesn't yell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it emits a warning though that this function won't run async as expected

Copy link
Member

@TylerLeonhardt TylerLeonhardt Feb 20, 2019

Choose a reason for hiding this comment

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

You could changed the method to return:

return await Task.FromResult(acc.ToArray());

to make it happy I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is a good idea because you might as well just suppress the warning. The compiler is happy, it is just kind to let us know that this other implementation will actually run synchronously despite the async declaration, Task.FromResult will still make it run sync but add unnecessary overhead. The warning is good to have because it is a way of tracking possible future improvement and also letting methods being called from this method know that they will actually need to adapt on a higher level if someone made them truly async as well

{
var acc = new List<CodeLens>();
foreach (SymbolReference sym in _symbolProvider.ProvideDocumentSymbols(scriptFile))
Expand Down
5 changes: 1 addition & 4 deletions src/PowerShellEditorServices/CodeLenses/ICodeLensProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using Microsoft.PowerShell.EditorServices.Utility;
using System.Collections.Generic;
using System.Management.Automation.Language;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -24,7 +21,7 @@ public interface ICodeLensProvider : IFeatureProvider
/// The document for which CodeLenses should be provided.
/// </param>
/// <returns>An array of CodeLenses.</returns>
CodeLens[] ProvideCodeLenses(ScriptFile scriptFile);
Task<CodeLens[]> ProvideCodeLenses(ScriptFile scriptFile);

/// <summary>
/// Resolves a CodeLens that was created without a Command.
Expand Down
4 changes: 1 addition & 3 deletions src/PowerShellEditorServices/CodeLenses/ICodeLenses.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.PowerShell.EditorServices.CodeLenses
Expand All @@ -29,6 +27,6 @@ public interface ICodeLenses
/// The document for which CodeLenses should be provided.
/// </param>
/// <returns>An array of CodeLenses.</returns>
CodeLens[] ProvideCodeLenses(ScriptFile scriptFile);
Task<CodeLens[]> ProvideCodeLenses(ScriptFile scriptFile);
}
}