Add functional and acceptance test harnesses and spl2 tests and bugfixes #102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… accepted already
fantavlik
commented
Aug 25, 2023
fantavlik
commented
Aug 25, 2023
| @@ -53,6 +53,7 @@ export async function installMissingSpl2Requirements(context: ExtensionContext, | |||
| } | |||
| } catch (err) { | |||
| reject(`Error retrieving configuration '${configKeyJavaPath}', err: ${err}`); | |||
| return Promise.resolve(); | |||
There was a problem hiding this comment.
These new Promise statements fix an issue @JasonConger and I saw during a demo of mine where the installation process can get stuck in a loop - we were not actually exiting the function after resolving the promise for a bunch of cases.
fantavlik
commented
Aug 25, 2023
| } | ||
| } else if (!javaLoc) { | ||
| } else if (!javaLoc && termsStatus !== TermsAcceptanceStatus.Accepted) { |
There was a problem hiding this comment.
Fixes and issue where we were prompting to accept terms even if you already had accepted them
fantavlik
commented
Aug 25, 2023
| try { | ||
| const javaVerCmd = child_process.spawnSync(javaLoc, ['-version'], { encoding : 'utf8' }); | ||
| if (!javaVerCmd || javaVerCmd.stdout) { | ||
| return false; | ||
| } | ||
| // java -version actually writes to stderr so check for a match there | ||
| const match = javaVerCmd.stderr.toString().match(/version \"([0-9]+)\.[0-9]+\.[0-9]\"/m); | ||
| output = javaVerCmd.stderr.toString(); | ||
| const match = output.match(/version \"([0-9]+)\.[0-9]+\.[0-9].*\"/m); |
There was a problem hiding this comment.
This fixes a linux issue where Java version of format openjdk version "17.0.8.1" was not being parsed correctly
fantavlik
commented
Aug 25, 2023
fantavlik
commented
Aug 25, 2023
| progressBar.text = `${progressBarText}...`; | ||
| await extract(zipfilePath, { dir: extractPath, onEntry: (entry, zipfile) => { | ||
| if (entry.fileName.endsWith(path.join('bin', 'java.exe'))) { | ||
| if (entry.fileName.endsWith('bin/java.exe') || entry.fileName.endsWith('bin\\java.exe')) { |
There was a problem hiding this comment.
This addresses a Windows installation issue found during testing!
fantavlik
changed the title
HaarisKhan
approved these changes
Aug 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added testing
Most of the Splunk Extension code depends on the
vscodepackage which requires a running instance of vscode to work. This PR adds testing harnesses using@vscode/test-electronusing these resources as guidance:Functional tests: these import the code directly so that individual functions can be tested similar to unit tests but with a running instance of vscode
Acceptance tests: these actually install the packaged
.vsixversion of the Extension into running VSCode instance in the testing environment. Unfortunately I wasn't able to get a meaningful test working yet with this but it was quite a lot of work just getting the harness working across platforms so I thought it would still be useful to work off of moving forward.Bugfixes
openjdk version "17.0.8.1"or similarjava.exebeing checked for Linux/Macresolve()Bump LSP
2.0.375