run benchmark at cursor (#972) #1303
Conversation
|
|
ramya-rao-a
force-pushed
the
master
branch
3 times, most recently
from
89e20b7
to
3d8a1f0
Compare
There was a problem hiding this comment.
@buyology Thanks for your patience, it took me a while to get to the PRs this last month.
Good start, I have left a few comments, mainly around refactoring some code. Please do take a look
src/testUtils.ts
Outdated
| * Get the test target arguments. | ||
| * | ||
| * @param testconfig Configuration for the Go extension. | ||
| */ | ||
| function targetArgs(testconfig: TestConfig): Thenable<Array<string>> { | ||
| function testTargetArgs(testconfig: TestConfig): Thenable<Array<string>> { | ||
| if (testconfig.functions) { | ||
| return Promise.resolve(['-run', util.format('^%s$', testconfig.functions.join('|'))]); |
There was a problem hiding this comment.
Instead of having a separate benchmarkTargetArgs, you can just use -bench here instead of -run if testconfig.isBenchmark is true
src/testUtils.ts
Outdated
| * Returns whether a given function name has a test prefix. | ||
| * Test functions have "Test" or "Example" as a prefix. | ||
| * | ||
| * @param the function name. | ||
| * @return whether the name has a test function prefix. | ||
| */ | ||
| function hasTestFunctionPrefix(name: string): boolean { | ||
| export function hasTestFunctionPrefix(name: string): boolean { |
There was a problem hiding this comment.
There was no real point of making this a separate function in the first place (my apologies).
Instead of exporting this prefix function and adding another for benchmark, we can simplify things as below
- remove
hasTestFunctionPrefixand move the prefix check intogetTestFunctions - create and export a new function
getBenchmarkFunctionswhich will have the same code asgetTestFunctionsbut with a different prefix check.
It will definitely get simpler when importing these functions in other files
src/goRunTestCodelens.ts
Outdated
|
|
||
| }); | ||
|
|
||
| return Promise.resolve(codelens); |
There was a problem hiding this comment.
This Promise should resolve only after both tests and benchmark functions are found
|
@buyology I went ahead and made the refactorings myself, hope you dont mind. |
|
@ramya-rao-a did not mind :) thanks! |
this is a stab at #972 along the lines of the discussion to have
run benchmarkavailable at the cursor levelnote:
-benchmemflag by default-run=^$to exclude all tests from being runlook and feel
(nb: this is my first foray into
typescript, and my first locs in javascript in many years so please bear with me)cc @scriptnull @ironcladlou @campoy @rakyll