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 support for Script, DeployReport, and DriftReport #106

Merged
merged 7 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ The definition of this GitHub Action is in [action.yml](https://github.com/Azure
path:

# optional when using a .sql script, required otherwise
# sqlpackage action on the .dacpac or .sqlproj file, only Publish is supported now
# sqlpackage action on the .dacpac or .sqlproj file, supported options are: Publish, Script, DeployReport, DriftReport
action:

# optional app (client) ID when using Azure Active Directory authentication
Expand Down
1 change: 1 addition & 0 deletions __testdata__/TestProject/sql-action.sqlproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
<Name>sql-action</Name>
<DSP>Microsoft.Data.Tools.Schema.Sql.SqlAzureV12DatabaseSchemaProvider</DSP>
<ModelCollation>1033, CI</ModelCollation>
<ProjectGuid>{829ec500-6bb6-42c3-a1ab-ab621099d153}</ProjectGuid>
</PropertyGroup>
</Project>
122 changes: 91 additions & 31 deletions __tests__/AzureSqlAction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,31 @@ jest.mock('fs');
describe('AzureSqlAction tests', () => {
afterEach(() => {
jest.restoreAllMocks();
})

it('executes dacpac action for DacpacAction type', async () => {
let inputs = getInputs(ActionType.DacpacAction) as IDacpacActionInputs;
let action = new AzureSqlAction(inputs);

let getSqlPackagePathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlPackagePath').mockResolvedValue('SqlPackage.exe');
let execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0);
});

await action.execute();
describe('validate sqlpackage calls for DacpacAction', () => {
const inputs = [
['Publish', '/TargetTimeout:20'],
['Script', '/DeployScriptPath:script.sql'],
['DriftReport', '/OutputPath:report.xml'],
['DeployReport', '/OutputPath:report.xml']
];

expect(getSqlPackagePathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledWith(`"SqlPackage.exe" /Action:Publish /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${inputs.filePath}" /TargetTimeout:20`);
it.each(inputs)('Validate %s action with args %s', async (actionName, sqlpackageArgs) => {
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to do it.each() instead of looping with a normal execution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is jest's syntax for testing against each row of inputs, kind of similar to NUnit's test cases.

let inputs = getInputsWithCustomSqlPackageAction(ActionType.DacpacAction, SqlPackageAction[actionName], sqlpackageArgs) as IDacpacActionInputs;
let action = new AzureSqlAction(inputs);

let getSqlPackagePathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlPackagePath').mockResolvedValue('SqlPackage.exe');
let execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0);

await action.execute();

expect(getSqlPackagePathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledWith(`"SqlPackage.exe" /Action:${actionName} /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${inputs.filePath}" ${sqlpackageArgs}`);
});
});

it('throws if SqlPackage.exe fails to publish dacpac', async () => {
let inputs = getInputs(ActionType.DacpacAction) as IDacpacActionInputs;
let action = new AzureSqlAction(inputs);
Expand Down Expand Up @@ -81,24 +90,33 @@ describe('AzureSqlAction tests', () => {
expect(await action.execute().catch(() => null)).rejects;
});

it('should build and publish database project', async () => {
const inputs = getInputs(ActionType.BuildAndPublish) as IBuildAndPublishInputs;
const action = new AzureSqlAction(inputs);
const expectedDacpac = path.join('./bin/Debug', 'TestProject.dacpac');

const parseCommandArgumentsSpy = jest.spyOn(DotnetUtils, 'parseCommandArguments').mockResolvedValue({});
const findArgumentSpy = jest.spyOn(DotnetUtils, 'findArgument').mockResolvedValue(undefined);
const getSqlPackagePathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlPackagePath').mockResolvedValue('SqlPackage.exe');
const execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0);
describe('validate build actions', () => {
const inputs = [
['Publish', '/p:DropPermissionsNotInSource=true'],
['Script', '/DeployScriptPath:script.sql'],
['DriftReport', '/OutputPath:report.xml'],
['DeployReport', '/OutputPath:report.xml']
];

await action.execute();
it.each(inputs)('Validate build and %s action with args %s', async (actionName, sqlpackageArgs) => {
const inputs = getInputsWithCustomSqlPackageAction(ActionType.BuildAndPublish, SqlPackageAction[actionName], sqlpackageArgs) as IBuildAndPublishInputs;
const action = new AzureSqlAction(inputs);
const expectedDacpac = path.join('./bin/Debug', 'TestProject.dacpac');

expect(parseCommandArgumentsSpy).toHaveBeenCalledTimes(1);
expect(findArgumentSpy).toHaveBeenCalledTimes(2);
expect(getSqlPackagePathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(2);
expect(execSpy).toHaveBeenNthCalledWith(1, `dotnet build "./TestProject.sqlproj" -p:NetCoreBuild=true --verbose --test "test value"`);
expect(execSpy).toHaveBeenNthCalledWith(2, `"SqlPackage.exe" /Action:Publish /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${expectedDacpac}"`);
const parseCommandArgumentsSpy = jest.spyOn(DotnetUtils, 'parseCommandArguments').mockResolvedValue({});
const findArgumentSpy = jest.spyOn(DotnetUtils, 'findArgument').mockResolvedValue(undefined);
const getSqlPackagePathSpy = jest.spyOn(AzureSqlActionHelper, 'getSqlPackagePath').mockResolvedValue('SqlPackage.exe');
const execSpy = jest.spyOn(exec, 'exec').mockResolvedValue(0);

await action.execute();

expect(parseCommandArgumentsSpy).toHaveBeenCalledTimes(1);
expect(findArgumentSpy).toHaveBeenCalledTimes(2);
expect(getSqlPackagePathSpy).toHaveBeenCalledTimes(1);
expect(execSpy).toHaveBeenCalledTimes(2);
expect(execSpy).toHaveBeenNthCalledWith(1, `dotnet build "./TestProject.sqlproj" -p:NetCoreBuild=true --verbose --test "test value"`);
expect(execSpy).toHaveBeenNthCalledWith(2, `"SqlPackage.exe" /Action:${actionName} /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${expectedDacpac}" ${sqlpackageArgs}`);
});
});

it('throws if dotnet fails to build sqlproj', async () => {
Expand Down Expand Up @@ -140,7 +158,7 @@ describe('AzureSqlAction tests', () => {
* @param connectionString The custom connection string to be used for the test. If not specified, a default one using SQL login will be used.
* @returns An ActionInputs objects based on the given action type.
*/
function getInputs(actionType: ActionType, connectionString: string = '') {
function getInputs(actionType: ActionType, connectionString: string = ''): IActionInputs {

const defaultConnectionString = 'Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder';
const config = connectionString ? new SqlConnectionConfig(connectionString) : new SqlConnectionConfig(defaultConnectionString);
Expand Down Expand Up @@ -168,7 +186,49 @@ function getInputs(actionType: ActionType, connectionString: string = '') {
actionType: ActionType.BuildAndPublish,
connectionConfig: config,
filePath: './TestProject.sqlproj',
buildArguments: '--verbose --test "test value"'
buildArguments: '--verbose --test "test value"',
sqlpackageAction: SqlPackageAction.Publish
} as IBuildAndPublishInputs
}
}
}

/**
* Gets test inputs used by SQL action based on actionType. Also accepts a custom SqlpackageAction type and additional arguments.
* @param actionType The action type used for testing
* @param sqlpackageAction The custom sqlpackage action type to test
* @param additionalArguments Additional arguments for this action type.
* @returns An ActionInputs objects based on the given action type.
*/
function getInputsWithCustomSqlPackageAction(actionType: ActionType, sqlpackageAction: SqlPackageAction, additionalArguments: string = ''): IActionInputs {
const defaultConnectionConfig = new SqlConnectionConfig('Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder');

switch(actionType) {
case ActionType.DacpacAction: {
return {
actionType: ActionType.DacpacAction,
connectionConfig: defaultConnectionConfig,
filePath: './TestPackage.dacpac',
sqlpackageAction: sqlpackageAction,
additionalArguments: additionalArguments
} as IDacpacActionInputs;
}
case ActionType.SqlAction: {
return {
actionType: ActionType.SqlAction,
connectionConfig: defaultConnectionConfig,
filePath: './TestFile.sql',
additionalArguments: additionalArguments
} as IActionInputs;
}
case ActionType.BuildAndPublish: {
return {
actionType: ActionType.BuildAndPublish,
connectionConfig: defaultConnectionConfig,
filePath: './TestProject.sqlproj',
buildArguments: '--verbose --test "test value"',
sqlpackageAction: sqlpackageAction,
additionalArguments: additionalArguments
} as IBuildAndPublishInputs
}
}
Expand Down
24 changes: 24 additions & 0 deletions __tests__/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,28 @@ describe('main.ts tests', () => {
expect(detectIPAddressSpy).not.toHaveBeenCalled();
expect(setFailedSpy).toHaveBeenCalledWith('Unable to find file at location');
})

describe('validate errors on unsupported sqlpackage action types', () => {
const inputs = [ ['Extract'], ['Export'], ['Import'], ['InvalidAction'] ];

it.each(inputs)('Throws for unsupported action %s', async (actionName) => {
let resolveFilePathSpy = jest.spyOn(AzureSqlActionHelper, 'resolveFilePath').mockReturnValue('./TestDacpacPackage.dacpac');
let setFailedSpy = jest.spyOn(core, 'setFailed');
let getInputSpy = jest.spyOn(core, 'getInput').mockImplementation((name, options) => {
switch(name) {
case 'connection-string': return 'Server=testServer.database.windows.net;Initial Catalog=testDB;User Id=testUser;Password=placeholder;';
case 'path': return './TestDacpacPackage.dacpac';
case 'action': return actionName;
default: return '';
}
});

await run();

expect(resolveFilePathSpy).toHaveBeenCalled();
expect(getInputSpy).toHaveBeenCalled();
expect(setFailedSpy).toHaveBeenCalled();
expect(setFailedSpy).toHaveBeenCalledWith(`Action ${actionName} is invalid. Supported action types are: Publish, Script, DriftReport, or DeployReport.`);
});
});
})
2 changes: 1 addition & 1 deletion lib/main.js

Large diffs are not rendered by default.

22 changes: 12 additions & 10 deletions src/AzureSqlAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export interface IBuildAndPublishInputs extends IActionInputs {
}

export enum SqlPackageAction {
// Only the Publish action is supported currently
Publish,
Extract,
Export,
Expand All @@ -53,15 +52,16 @@ export default class AzureSqlAction {
await this._executeSqlFile(this._inputs);
}
else if (this._inputs.actionType === ActionType.BuildAndPublish) {
const dacpacPath = await this._executeBuildProject(this._inputs as IBuildAndPublishInputs);
const buildAndPublishInputs = this._inputs as IBuildAndPublishInputs;
Copy link
Member

Choose a reason for hiding this comment

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

Does anything need to be handled in case this doesn't cast successfully?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any invalid inputs should be handled by this point

const dacpacPath = await this._executeBuildProject(buildAndPublishInputs);

// Reuse DacpacAction for publish
const publishInputs = {
actionType: ActionType.DacpacAction,
connectionConfig: this._inputs.connectionConfig,
connectionConfig: buildAndPublishInputs.connectionConfig,
filePath: dacpacPath,
additionalArguments: this._inputs.additionalArguments,
sqlpackageAction: SqlPackageAction.Publish
additionalArguments: buildAndPublishInputs.additionalArguments,
sqlpackageAction: buildAndPublishInputs.sqlpackageAction
} as IDacpacActionInputs;
await this._executeDacpacAction(publishInputs);
}
Expand Down Expand Up @@ -164,13 +164,15 @@ export default class AzureSqlAction {
let args = '';

switch (inputs.sqlpackageAction) {
case SqlPackageAction.Publish: {
args += `/Action:Publish /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${inputs.filePath}"`;
case SqlPackageAction.Publish:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about leveraging getSqlpackageActionTypeFromString(), saving the result into a variable, and switching on that variable? Then we wouldn't have to have logic checking for no action being passed in in two different places.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I removed the redundant logic of checking for no action passed in here. SqlPackageAction should be set already at this point coming from the earlier check.

case SqlPackageAction.Script:
case SqlPackageAction.DeployReport:
case SqlPackageAction.DriftReport:
args += `/Action:${SqlPackageAction[inputs.sqlpackageAction]} /TargetConnectionString:"${inputs.connectionConfig.ConnectionString}" /SourceFile:"${inputs.filePath}"`;
break;
}
default: {

default:
throw new Error(`Not supported SqlPackage action: '${SqlPackageAction[inputs.sqlpackageAction]}'`);
}
}

if (!!inputs.additionalArguments) {

Choose a reason for hiding this comment

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

what happens if the user doesn't pass in the OutputPath or DeployScriptPath for these new actions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sqlpackage will error out asking for one of those to be specified

Expand Down
19 changes: 18 additions & 1 deletion src/AzureSqlActionHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,28 @@ export default class AzureSqlActionHelper {
}

public static getSqlpackageActionTypeFromString(action: string): SqlPackageAction {
// Default to Publish if not specified
if (!action) {
return SqlPackageAction.Publish;
}

switch (action.trim().toLowerCase()) {
case 'publish':
return SqlPackageAction.Publish;
// case 'extract':
// return SqlPackageAction.Extract;
// case 'import':
// return SqlPackageAction.Import;
// case 'export':
// return SqlPackageAction.Export;
case 'driftreport':
return SqlPackageAction.DriftReport;
case 'deployreport':
return SqlPackageAction.DeployReport;
case 'script':
return SqlPackageAction.Script;
default:
throw new Error(`Action ${action} is invalid. Supported action types are: Publish.`);
throw new Error(`Action ${action} is invalid. Supported action types are: Publish, Script, DriftReport, or DeployReport.`);
}
}

Expand Down