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

Use yaml package when selecting build configs #351

Merged
merged 8 commits into from
Apr 22, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 2 additions & 0 deletions extension.bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@
// The tests should import '../extension.bundle'. At design-time they live in tests/ and so will pick up this file (extension.bundle.ts).
// At runtime the tests live in dist/tests and will therefore pick up the main webpack bundle at dist/extension.bundle.js.
export * from 'vscode-azureextensionui';
export { tryGetSelection } from './src/commands/openYAMLConfigFile';
// Export activate/deactivate for main.js
export { activateInternal, deactivateInternal } from './src/extension';
export * from './src/extensionVariables';
export { BuildConfig } from './src/tree/localProject/ConfigGroupTreeItem';

// NOTE: The auto-fix action "source.organizeImports" does weird things with this file, but there doesn't seem to be a way to disable it on a per-file basis so we'll just let it happen
74 changes: 64 additions & 10 deletions src/commands/openYAMLConfigFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
*--------------------------------------------------------------------------------------------*/

import { basename } from 'path';
import { Position, Range, TextDocument, window, workspace } from "vscode";
import { Position, Range, TextDocument, window, workspace } from 'vscode';
import { IActionContext, IAzureQuickPickItem } from "vscode-azureextensionui";
import { CST, Document, parseDocument } from 'yaml';
import { Pair, Scalar, YAMLMap, YAMLSeq } from 'yaml/types';
import { ext } from "../extensionVariables";
import { EnvironmentTreeItem } from "../tree/EnvironmentTreeItem";
import { BuildConfig, GitHubConfigGroupTreeItem } from '../tree/localProject/ConfigGroupTreeItem';
Expand Down Expand Up @@ -41,18 +43,70 @@ export async function openYAMLConfigFile(context: IActionContext, node?: StaticW
}

const configDocument: TextDocument = await workspace.openTextDocument(yamlFilePath);
const selection: Range | undefined = buildConfigToSelect ? await getSelection(configDocument, buildConfigToSelect) : undefined;
const selection: Range | undefined = buildConfigToSelect ? await tryGetSelection(configDocument, buildConfigToSelect) : undefined;
await window.showTextDocument(configDocument, { selection });
}

async function getSelection(configDocument: TextDocument, buildConfigToSelect: BuildConfig): Promise<Range | undefined> {
const configRegex: RegExp = new RegExp(`${buildConfigToSelect}:`);
export async function tryGetSelection(configDocument: TextDocument, buildConfigToSelect: BuildConfig): Promise<Range | undefined> {
const configDocumentText: string = configDocument.getText();
const buildConfigRegex: RegExp = new RegExp(`${buildConfigToSelect}:`, 'g');
const buildConfigMatches: RegExpMatchArray | null = configDocumentText.match(buildConfigRegex);

let offset: number = configDocument.getText().search(configRegex);
// Shift the offset to the beginning of the build config's value
offset += `${buildConfigToSelect}: `.length;
if (buildConfigMatches && buildConfigMatches.length > 1) {
void ext.ui.showWarningMessage(localize('foundMultipleBuildConfigs', 'Multiple "{0}" build configurations were found in "{1}".', buildConfigToSelect, basename(configDocument.uri.fsPath)));
return undefined;
}

try {
type YamlNode = YAMLMap | YAMLSeq | Pair | Scalar | undefined | null;
const yamlNodes: YamlNode[] = [];
const parsedYaml: Document.Parsed = parseDocument(configDocumentText, { keepCstNodes: true });
let yamlNode: YamlNode = parsedYaml.contents;

while (yamlNode) {
if ('key' in yamlNode && (<Scalar>yamlNode.key).value === buildConfigToSelect && 'value' in yamlNode) {
const cstNode: CST.Node | undefined = (<Scalar>yamlNode.value)?.cstNode;
const range = cstNode?.rangeAsLinePos;

if (range && range.end) {
// Range isn't zero-indexed by default
range.start.line--;
range.start.col--;
range.end.line--;
range.end.col--;

if (cstNode?.comment) {
// The end range includes the length of the comment
range.end.col -= cstNode.comment.length + 1;

const lineText: string = (configDocument.lineAt(range.start.line)).text;

// Don't include the comment character
if (lineText[range.end.col] === '#') {
range.end.col--;
}

// Don't include any horizontal whitespace between the end of the YAML value and the comment
while (/[ \t]/.test(lineText[range.end.col - 1])) {
range.end.col--;
}
}

const startPosition: Position = new Position(range.start.line, range.start.col);
const endPosition: Position = new Position(range.end.line, range.end.col);
return configDocument.validateRange(new Range(startPosition, endPosition));
}
} else if ('items' in yamlNode) {
yamlNodes.push(...yamlNode.items)
} else if ('value' in yamlNode && typeof yamlNode.value === 'object') {
yamlNodes.push(yamlNode.value)
}

yamlNode = yamlNodes.pop();
}
} catch {
// Ignore errors
}

const position: Position = configDocument.positionAt(offset);
const configValueRegex: RegExp = /['"].*['"]/;
ejizba marked this conversation as resolved.
Show resolved Hide resolved
return configDocument.getWordRangeAtPosition(position, configValueRegex);
return undefined;
}
7 changes: 0 additions & 7 deletions test/global.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,3 @@ suiteSetup(async function (this: IHookCallbackContext): Promise<void> {

longRunningTestsEnabled = !/^(false|0)?$/i.test(process.env.ENABLE_LONG_RUNNING_TESTS || '');
});

suite('suite1', () => {
test('test1', () => {
// suiteSetup only runs if a suite/test exists, so added a placeholder test here so we can at least verify the extension can activate
// once actual tests exist, we can remove this
});
});
157 changes: 157 additions & 0 deletions test/selectBuildConfigs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE.md in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { Position, Range, TextDocument, TextDocumentContentProvider, Uri, workspace } from 'vscode';
import { BuildConfig, tryGetSelection } from "../extension.bundle";

interface ISelectBuildConfigTestCase {
workflowIndex: number;
buildConfig: BuildConfig;
expectedSelection: undefined | {
Copy link
Member

Choose a reason for hiding this comment

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

nit: I don't think I've ever seen anyone list undefined first in a | type haha

line: number;
startChar: number;
endChar: number;
}
}

suite('Select Build Configurations in GitHub Workflow Files', () => {
const testCases: ISelectBuildConfigTestCase[] = [
{ workflowIndex: 0, buildConfig: 'api_location', expectedSelection: { line: 7, startChar: 24, endChar: 26 } },
{ workflowIndex: 0, buildConfig: 'app_location', expectedSelection: { line: 6, startChar: 24, endChar: 27 } },
{ workflowIndex: 0, buildConfig: 'output_location', expectedSelection: { line: 8, startChar: 27, endChar: 29 } },
{ workflowIndex: 0, buildConfig: 'app_artifact_location', expectedSelection: undefined },

{ workflowIndex: 1, buildConfig: 'api_location', expectedSelection: { line: 7, startChar: 24, endChar: 38 } },
{ workflowIndex: 1, buildConfig: 'app_location', expectedSelection: { line: 6, startChar: 24, endChar: 38 } },
{ workflowIndex: 1, buildConfig: 'output_location', expectedSelection: undefined },
{ workflowIndex: 1, buildConfig: 'app_artifact_location', expectedSelection: { line: 8, startChar: 33, endChar: 54 }},

{ workflowIndex: 2, buildConfig: 'api_location', expectedSelection: undefined },
{ workflowIndex: 2, buildConfig: 'app_location', expectedSelection: undefined },
{ workflowIndex: 2, buildConfig: 'output_location', expectedSelection: undefined },
{ workflowIndex: 2, buildConfig: 'app_artifact_location', expectedSelection: undefined },

{ workflowIndex: 3, buildConfig: 'api_location', expectedSelection: { line: 30, startChar: 24, endChar: 39 }},
{ workflowIndex: 3, buildConfig: 'app_location', expectedSelection: { line: 29, startChar: 24, endChar: 57 }},
{ workflowIndex: 3, buildConfig: 'output_location', expectedSelection: { line: 31, startChar: 27, endChar: 54 }},
{ workflowIndex: 3, buildConfig: 'app_artifact_location', expectedSelection: undefined },
];

const workflowProvider: TextDocumentContentProvider = new (class implements TextDocumentContentProvider {
provideTextDocumentContent(uri: Uri): string {
return workflows[parseInt(uri.path)];
}
})();
const scheme: string = 'testWorkflows';
workspace.registerTextDocumentContentProvider(scheme, workflowProvider);

for (const testCase of testCases) {
const title: string = `Workflow ${testCase.workflowIndex}: ${testCase.buildConfig}`;

test(title, async () => {
const uri: Uri = Uri.parse(`${scheme}:${testCase.workflowIndex}`);
const configDocument: TextDocument = await workspace.openTextDocument(uri);
const selection: Range | undefined = await tryGetSelection(configDocument, testCase.buildConfig);
let expectedSelection: Range | undefined;

if (testCase.expectedSelection) {
const expectedStart: Position = new Position(testCase.expectedSelection.line, testCase.expectedSelection.startChar);
const expectedEnd: Position = new Position(testCase.expectedSelection.line, testCase.expectedSelection.endChar);
expectedSelection = new Range(expectedStart, expectedEnd);
}

assert.ok(expectedSelection && selection?.isEqual(expectedSelection) || selection === expectedSelection, 'Actual and expected selections do not match');
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use assert.strictEquals() here. Using assert.ok seems kind of roundabout 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 went with assert.ok because comparing two ranges uses their isEqual() function. I updated the assert to hopefully make it look more purposeful

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, okay, makse sense.

});
}
});

const workflows: string[] = [
`jobs:
build_and_deploy_job:
steps:
- uses: Azure/static-web-apps-deploy@v0.0.1-preview
id: builddeploy
with:
app_location: "/"
api_location: ""
output_location: ""`,

`jobs:
build_and_deploy_job:
steps:
- uses: Azure/static-web-apps-deploy@v0.0.1-preview
id: builddeploy
with:
app_location: "app/location"
api_location: 'api/location'
app_artifact_location: app/artifact/location`,

`jobs:
build_and_deploy_job1:
steps:
- uses: Azure/static-web-apps-deploy@v0.0.1-preview
id: builddeploy
with:
app_location: "src"
api_location: "api"
output_location: "build"

build_and_deploy_job2:
steps:
- uses: Azure/static-web-apps-deploy@v0.0.1-preview
id: builddeploy
with:
app_location: "src"
api_location: "api"
output_location: "build"`,

`name: Azure Static Web Apps CI/CD

on:
push:
branches:
- master
pull_request:
types: [opened, synchronize, reopened, closed]
branches:
- master

jobs:
build_and_deploy_job:
if: github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.action != 'closed')
runs-on: ubuntu-latest
name: Build and Deploy Job
steps:
- uses: actions/checkout@v2
with:
submodules: true
- name: Build And Deploy
id: builddeploy
uses: Azure/static-web-apps-deploy@v0.0.1-preview
with:
azure_static_web_apps_api_token: $\{{ secrets.AZURE_STATIC_WEB_APPS_API_TOKEN_AMBITIOUS_ROCK_0D992521E }}
repo_token: $\{{ secrets.GITHUB_TOKEN }} # Used for Github integrations (i.e. PR comments)
action: "upload"
###### Repository/Build Configurations - These values can be configured to match your app requirements. ######
# For more information regarding Static Web App workflow configurations, please visit: https://aka.ms/swaworkflowconfig
app_location: "super/long/path/to/app/location" # There are tabs before this comment
api_location: 'single/quotes' #There are spaces before this comment
output_location: output/location with/spaces # There is a single space before this comment
###### End of Repository/Build Configurations ######

close_pull_request_job:
if: github.event_name == 'pull_request' && github.event.action == 'closed'
runs-on: ubuntu-latest
name: Close Pull Request Job
steps:
- name: Close Pull Request
id: closepullrequest
uses: Azure/static-web-apps-deploy@v0.0.1-preview
with:
azure_static_web_apps_api_token: $\{{ secrets.AZURE_STATIC_WEB_APPS_API_TOKEN_AMBITIOUS_ROCK_0D992521E }}
action: "close"

`];
Copy link
Member

Choose a reason for hiding this comment

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

Could we try location paths with special characters and maybe some foreign characters? (Chinese or Korean or something seems plausible)