Skip to content

Commit

Permalink
feat: do not bail on error (#708)
Browse files Browse the repository at this point in the history
  • Loading branch information
AnWeber committed Aug 4, 2024
1 parent 82ee753 commit 7981fe0
Show file tree
Hide file tree
Showing 29 changed files with 164 additions and 121 deletions.
1 change: 1 addition & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
"send",
"${file}",
"--all",
"--bail",
"-o",
"short"
],
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- only try loading grpc reflection if no other protodefinition file is imported or it is explicitly triggerd using grpcReflection (#757)
- add better handling for disabled and error tests (Anweber/vscode-httpyac#297, #760)
- support empty passwords for Basic auth (#751)
- error do not bail test runs and instead just set error for this httpRegion (#708)

## [6.14.0] ( 2024-06-01)
### Features
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
"google-protobuf": "^3.21.2",
"got": "^11.8.6",
"grpc-js-reflection-client": "^1.2.14",
"hookpoint": "4.0.1",
"hookpoint": "4.1.0",
"http-proxy-agent": "^7.0.2",
"https-proxy-agent": "^7.0.4",
"inquirer": "^9.2.20",
Expand Down
6 changes: 4 additions & 2 deletions src/cli/send/jsonOutput.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,10 @@ export function createTestSummary(requests: Array<SendOutputRequest>): SendReque
totalRequests: requests.length,
skippedRequests: requests.filter(obj => obj.summary.skippedTests > 0).length,
failedRequests: requests.filter(obj => obj.summary.failedTests > 0).length,
successRequests: requests.filter(obj => obj.summary.failedTests === 0).length,
erroredRequests: requests.filter(obj => obj.summary.erroredTests === 0).length,
successRequests: requests.filter(
obj => obj.summary.failedTests + obj.summary.erroredTests + obj.summary.skippedTests === 0
).length,
erroredRequests: requests.filter(obj => obj.summary.erroredTests > 0).length,
totalTests: requests.map(obj => obj.summary.totalTests).reduce(sum, 0),
failedTests: requests.map(obj => obj.summary.failedTests).reduce(sum, 0),
successTests: requests.map(obj => obj.summary.successTests).reduce(sum, 0),
Expand Down
19 changes: 17 additions & 2 deletions src/cli/send/plugin/bailOnFailedTestInterceptor.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,31 @@
import { HookTriggerContext } from 'hookpoint';

import * as models from '../../../models';
import { addSkippedTestResult } from '../../../utils';

let bailInBeforeLoop = false;

export const bailOnFailedTestInterceptor = {
id: 'bailOnFailed',
beforeLoop: async function beforeLoop(hookContext: { args: [models.ProcessorContext] }) {
if (bailInBeforeLoop) {
const [context] = hookContext.args;
addSkippedTestResult(context.httpRegion, 'request skipped because of bail');
return false;
}
return true;
},
onError: async function bailOnError() {
bailInBeforeLoop = true;
return true;
},
afterTrigger: async function bail(hookContext: HookTriggerContext<[models.ProcessorContext], boolean>) {
const context = hookContext.args[0];
const failedTest = context.httpRegion.testResults?.find?.(obj =>
[models.TestResultStatus.FAILED, models.TestResultStatus.ERROR].includes(obj.status)
[models.TestResultStatus.FAILED].includes(obj.status)
);
if (failedTest) {
throw failedTest.error || new Error('bail on failed test');
bailInBeforeLoop = true;
}
return true;
},
Expand Down
12 changes: 9 additions & 3 deletions src/cli/send/plugin/testExitCodeInterceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@ import * as models from '../../../models';

export const testExitCodeInterceptor = {
id: 'testExitCode',

onError: async function onError(): Promise<boolean | undefined> {
process.exitCode = 10;
return true;
},

afterTrigger: async function bail(hookContext: HookTriggerContext<[models.ProcessorContext], boolean>) {
const context = hookContext.args[0];
const failedTest = context.httpRegion.testResults?.some?.(obj =>
[models.TestResultStatus.FAILED, models.TestResultStatus.ERROR].includes(obj.status)
const hasFailedTestResult = context.httpRegion.testResults?.some?.(
obj => obj.status === models.TestResultStatus.FAILED
);
if (failedTest) {
if (hasFailedTestResult) {
process.exitCode = 20;
}
return true;
Expand Down
16 changes: 15 additions & 1 deletion src/cli/send/send.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,22 @@ function reportOutput(context: Omit<models.HttpFileSendContext, 'httpFile'>, opt
console.info(transformToJunit(cliJsonOutput));
} else if (context.scriptConsole) {
context.scriptConsole.info('');

const requestCounts: Array<string> = [];
if (cliJsonOutput.summary.successRequests > 0) {
requestCounts.push(chalk`{green ${cliJsonOutput.summary.successRequests} succeeded}`);
}
if (cliJsonOutput.summary.failedRequests > 0) {
requestCounts.push(chalk`{red ${cliJsonOutput.summary.failedRequests} failed}`);
}
if (cliJsonOutput.summary.erroredRequests > 0) {
requestCounts.push(chalk`{red ${cliJsonOutput.summary.erroredRequests} errored}`);
}
if (cliJsonOutput.summary.skippedRequests > 0) {
requestCounts.push(chalk`{yellow ${cliJsonOutput.summary.skippedRequests} skipped}`);
}
context.scriptConsole.info(
chalk`{bold ${cliJsonOutput.summary.totalRequests}} requests processed ({green ${cliJsonOutput.summary.successRequests} succeeded}, {red ${cliJsonOutput.summary.failedRequests} failed})`
chalk`{bold ${cliJsonOutput.summary.totalRequests}} requests processed (${requestCounts.join(', ')}))`
);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/assert/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export * from './provideAssertValueHeader';
export * from './provideAssertValueJavascript';
export * from './provideAssertValueStatus';
export * from './registerAssertPlugin';
export * from './testFailedInterceptor';
2 changes: 2 additions & 0 deletions src/plugins/assert/registerAssertPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import { provideAssertValueDuration } from './provideAssertValueDuration';
import { provideAssertValueHeader } from './provideAssertValueHeader';
import { provideAssertValueJavascript } from './provideAssertValueJavascript';
import { provideAssertValueStatus } from './provideAssertValueStatus';
import { TestFailedInterceptor } from './testFailedInterceptor';

export function registerAssertPlugin(api: models.HttpyacHooksApi) {
api.hooks.execute.addInterceptor(new TestFailedInterceptor());
api.hooks.provideAssertValue.addHook('status', provideAssertValueStatus, { before: ['request'] });
api.hooks.provideAssertValue.addHook('header', provideAssertValueHeader, { before: ['request'] });
api.hooks.provideAssertValue.addHook('javascript', provideAssertValueJavascript, { before: ['request'] });
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/assert/test/assert.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,6 @@ describe('assert', () => {
});
expect(responses.length).toBe(1);
expect(responses[0].statusCode).toBe(200);
expect(httpFile.httpRegions[0].testResults?.length).toBeUndefined();
expect(httpFile.httpRegions[0].testResults?.length).toBe(0);
});
});
24 changes: 24 additions & 0 deletions src/plugins/assert/testFailedInterceptor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { HookInterceptor, HookTriggerContext } from 'hookpoint';

import * as models from '../../models';
import { addTestResultToHttpRegion, isError, parseError } from '../../utils';

export class TestFailedInterceptor implements HookInterceptor<[models.ProcessorContext], boolean | void> {
id = 'testFailed';
before = ['processedHttpRegion'];

public async onError(
err: unknown,
hookContext: HookTriggerContext<[models.ProcessorContext], boolean | undefined>
): Promise<boolean | undefined> {
if (isError(err)) {
const { httpRegion } = hookContext.args[0];
addTestResultToHttpRegion(httpRegion, {
message: err.message,
error: parseError(err),
status: models.TestResultStatus.ERROR,
});
}
return true;
}
}
16 changes: 13 additions & 3 deletions src/plugins/core/execute/processedHttpRegionInterceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,34 @@ export class ProcessedHttpRegionInterceptor implements HookInterceptor<[models.P
return true;
}

public async onError(
_err: unknown,
hookContext: HookTriggerContext<[models.ProcessorContext], boolean | undefined>
): Promise<boolean | undefined> {
this.updateProcessedHttpRegionAfterLoop(hookContext.args[0]);
return true;
}

public async afterLoop(
hookContext: HookTriggerContext<[models.ProcessorContext], boolean | undefined>
): Promise<boolean | undefined> {
const [context] = hookContext.args;
this.updateProcessedHttpRegionAfterLoop(hookContext.args[0]);
return true;
}

private updateProcessedHttpRegionAfterLoop(context: models.ProcessorContext) {
const processedHttpRegion = context.processedHttpRegions
?.slice()
.reverse()
?.find(obj => obj.id === context.httpRegion.id);
if (processedHttpRegion) {
processedHttpRegion.end = performance.now();
processedHttpRegion.duration = processedHttpRegion.end - processedHttpRegion.start;
processedHttpRegion.disabled = !!hookContext.bail;
processedHttpRegion.metaData = {
...context.httpRegion.metaData,
};
processedHttpRegion.testResults = context.httpRegion.testResults;
}
return true;
}

private async afterResponseLoggingLoop(
Expand All @@ -66,6 +75,7 @@ export class ProcessedHttpRegionInterceptor implements HookInterceptor<[models.P
filename: context.httpFile.fileName,
symbol: context.httpRegion.symbol,
isGlobal: context.httpRegion.isGlobal(),
testResults: context.httpRegion.testResults,
start: performance.now(),
};
}
Expand Down
27 changes: 12 additions & 15 deletions src/plugins/core/test/metadata/noRejectUnauthorized.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { generateCACertificate, getLocal, Mockttp } from 'mockttp';

import { initFileProvider, parseHttp, sendHttp, sendHttpFile } from '../../../../test/testUtils';
import { TestResultStatus } from '../../../../models';

describe('metadata.noRejectUnauthorized', () => {
let localServer: Mockttp;
Expand All @@ -14,24 +15,20 @@ describe('metadata.noRejectUnauthorized', () => {
});
afterAll(async () => await localServer.stop());
it('should throw self signed certificate error', async () => {
try {
initFileProvider();
await localServer.forGet('/selfsignederror').thenReply(200);
initFileProvider();

await sendHttp(
`
const httpFile = await parseHttp(`
GET /selfsignederror
`,
{
host: `https://localhost:${localServer.port}`,
}
);
`);
await localServer.forGet('/selfsignederror').thenReply(200);

throw new Error('no error while sendhttp');
} catch (err) {
expect(err instanceof Error && err.name).toBe('RequestError');
expect(err.toString()).toContain('signed certificate in certificate chain');
}
await sendHttpFile({
httpFile,
variables: {
host: `https://localhost:${localServer.port}`,
},
});
expect(httpFile.httpRegions[0].testResults?.some(t => t.status === TestResultStatus.ERROR)).toBeTruthy();
});
it('should use metadata tag', async () => {
initFileProvider();
Expand Down
13 changes: 5 additions & 8 deletions src/plugins/core/test/metadata/ref.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,18 @@ foo={{foo.foo}}
initFileProvider();
const httpFile = await parseHttp(`
###
# @ref not_found
POST /post?test={{foo.test}}
foo={{foo.foo}}
`);

await expect(
async () =>
await send({
httpFile,
httpRegion: httpFile.httpRegions[1],
})
).rejects.toThrow(`ref not_found not found`);
await send({
httpFile,
httpRegion: httpFile.httpRegions[1],
});
expect(httpFile.httpRegions?.[0]?.testResults?.[0].message).toBe(`ref not_found not found`);
});

it('name + ref + falsy body', async () => {
Expand Down
12 changes: 4 additions & 8 deletions src/plugins/intellij/intellijAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export class IntellijAction {
async processOnRequest(_request: models.Request, context: models.ProcessorContext): Promise<void> {
utils.report(context, 'execute Intellij Javascript');
const scriptData = await this.loadScript(context);
if (!scriptData || this.isStreamingScript(scriptData)) {
if (this.isStreamingScript(scriptData)) {
return;
}

Expand All @@ -38,9 +38,6 @@ export class IntellijAction {

async processOnStreaming(context: models.ProcessorContext): Promise<void> {
const scriptData = await this.loadScript(context);
if (!scriptData) {
return;
}
this.scriptData = scriptData;
if (this.isStreamingScript(scriptData) && context.requestClient) {
const requestClient = context.requestClient;
Expand All @@ -55,7 +52,7 @@ export class IntellijAction {
async processOnResponse(response: models.HttpResponse, context: models.ProcessorContext): Promise<void> {
utils.report(context, 'execute Intellij Javascript');
const scriptData = await this.loadScript(context);
if (!scriptData || this.isStreamingScript(scriptData)) {
if (this.isStreamingScript(scriptData)) {
return;
}

Expand Down Expand Up @@ -83,7 +80,7 @@ export class IntellijAction {
return false;
}

private async loadScript(context: models.ProcessorContext): Promise<models.ScriptData | undefined> {
private async loadScript(context: models.ProcessorContext): Promise<models.ScriptData> {
if (this.isIntellijScriptData(this.scriptData)) {
try {
return {
Expand All @@ -94,9 +91,8 @@ export class IntellijAction {
lineOffset: 0,
};
} catch (err) {
io.userInteractionProvider.showErrorMessage?.(`error loading script ${this.scriptData.fileName}`);
(context.scriptConsole || io.log).error(this.scriptData.fileName, err);
return undefined;
throw new Error(`error loading script ${this.scriptData.fileName}`);
}
} else {
return this.scriptData;
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/javascript/moduleUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ function deleteVariables(contextKeys: string[], context: vm.Context, deleteVaria
deleteVariable(key);
}
} catch (err) {
log.info(err);
log.info(`error on delete of ${key}`, err);
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions src/plugins/oauth2/flow/authorizationCodeFlow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ class AuthorizationCodeFlow implements OpenIdFlow {
}

getCacheKey(config: models.OpenIdConfiguration) {
if (assertConfiguration(config, ['tokenEndpoint', 'authorizationEndpoint', 'clientId'])) {
return `authorization_code_${config.variablePrefix}_${config.clientId}_${config.tokenEndpoint}`;
}
return false;
assertConfiguration(config, ['tokenEndpoint', 'authorizationEndpoint', 'clientId']);
return `authorization_code_${config.variablePrefix}_${config.clientId}_${config.tokenEndpoint}`;
}

async perform(
Expand Down
6 changes: 2 additions & 4 deletions src/plugins/oauth2/flow/clientCredentialsFlow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,8 @@ class ClientCredentialsFlow implements OpenIdFlow {
}

getCacheKey(config: models.OpenIdConfiguration) {
if (assertConfiguration(config, ['tokenEndpoint', 'clientId', 'clientSecret'])) {
return `client_credentials_${config.variablePrefix}_${config.clientId}_${config.tokenEndpoint}`;
}
return false;
assertConfiguration(config, ['tokenEndpoint', 'clientId', 'clientSecret']);
return `client_credentials_${config.variablePrefix}_${config.clientId}_${config.tokenEndpoint}`;
}

async perform(
Expand Down
Loading

0 comments on commit 7981fe0

Please sign in to comment.