Skip to content

Record and playback - POC for skip functionality in common recorder #4336

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.

Sign up for GitHub

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

Closed
HarshaNalluru wants to merge 2 commits into from
Closed

Record and playback - POC for skip functionality in common recorder #4336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89e6135
update recorder.skip functionality
HarshaNalluru Jul 17, 2019
0005332
simplify nested ifs
HarshaNalluru Jul 17, 2019
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
6 changes: 6 additions & 0 deletions ...orage-queue/recordings/browsers/aborter/recording_should_abort_after_aborter_timeout.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"recordings": [],
"uniqueTestInfo": {
"queue": "queue156332152913504024"
}
}
6 changes: 6 additions & 0 deletions .../recordings/browsers/aborter/recording_should_abort_after_parent_aborter_calls_abort.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"recordings": [],
"uniqueTestInfo": {
"queue": "queue156332152914004887"
}
}
6 changes: 6 additions & 0 deletions ...ueue/recordings/browsers/aborter/recording_should_abort_after_parent_aborter_timeout.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"recordings": [],
"uniqueTestInfo": {
"queue": "queue156332152914507000"
}
}
6 changes: 6 additions & 0 deletions ...s/browsers/aborter/recording_should_abort_when_calling_abort_before_request_finishes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"recordings": [],
"uniqueTestInfo": {
"queue": "queue156332152881302622"
}
}
4 changes: 4 additions & 0 deletions sdk/storage/storage-queue/test/aborter.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe("Aborter", () => {
});

it("should abort when calling abort() before request finishes", async () => {
recorder.skip("browser");
const aborter = Aborter.none;
const response = queueClient.create({ abortSignal: aborter });
aborter.abort();
Expand All @@ -54,13 +55,15 @@ describe("Aborter", () => {
});

it("should abort after aborter timeout", async () => {
recorder.skip("browser");
try {
await queueClient.create({ abortSignal: Aborter.timeout(1) });
assert.fail();
} catch (err) {}
});

it("should abort after parent aborter calls abort()", async () => {
recorder.skip("browser");
try {
const aborter = Aborter.none;
const response = queueClient.create({ abortSignal: aborter.withTimeout(10 * 60 * 1000) });
Expand All @@ -71,6 +74,7 @@ describe("Aborter", () => {
});

it("should abort after parent aborter timeout", async () => {
recorder.skip("browser");
try {
const aborter = Aborter.timeout(1);
const response = queueClient.create({ abortSignal: aborter.withTimeout(10 * 60 * 1000) });
Expand Down
56 changes: 38 additions & 18 deletions sdk/storage/storage-queue/test/utils/recorder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,18 @@ export function delay(milliseconds: number): Promise<void> | null {
* * Tempfile: the request makes use of a random tempfile created locally, and the recorder does not support recording it as unique information
* * UUID: a UUID is randomly generated within the SDK and used in an HTTP request, resulting in Nock being unable to recognize it
*/
const skip = [
// Abort
"browsers/aborter/recording_should_abort_after_aborter_timeout.json",
// Abort
"browsers/aborter/recording_should_abort_after_parent_aborter_calls_abort.json",
// Abort
"browsers/aborter/recording_should_abort_after_parent_aborter_timeout.json",
// Abort
"browsers/aborter/recording_should_abort_when_calling_abort_before_request_finishes.json",
// Character
"browsers/messagesurl/recording_enqueue_peek_dequeue_special_characters.json"
];
// const skip = [
// // Abort
// "browsers/aborter/recording_should_abort_after_aborter_timeout.json",
// // Abort
// "browsers/aborter/recording_should_abort_after_parent_aborter_calls_abort.json",
// // Abort
// "browsers/aborter/recording_should_abort_after_parent_aborter_timeout.json",
// // Abort
// "browsers/aborter/recording_should_abort_when_calling_abort_before_request_finishes.json",
// // Character
// "browsers/messagesurl/recording_enqueue_peek_dequeue_special_characters.json"
// ];

abstract class Recorder {
protected readonly filepath: string;
Expand Down Expand Up @@ -153,9 +153,9 @@ abstract class Recorder {
return updatedRecording;
}

public skip(): boolean {
return skip.includes(this.filepath);
}
// public skip(): boolean {
Copy link
Copy Markdown
Member

@XiaoningLiu XiaoningLiu Jul 18, 2019

Choose a reason for hiding this comment

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

Do we still need commentted code here?

Copy link
Copy Markdown
Contributor Author

@HarshaNalluru HarshaNalluru Jul 18, 2019

Choose a reason for hiding this comment

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

No, this will be deleted.
This current PR is just to show how the skip list can be avoided and hear some feedback for it.
Instead of doing the updates in recorder.ts files, I'm planning to update the new common recorder #4281.

// return skip.includes(this.filepath);
// }

public abstract record(): void;
public abstract playback(): void;
Expand Down Expand Up @@ -436,9 +436,9 @@ export function record(testContext: any) {
recorder = new NockRecorder(testHierarchy, testTitle);
}

if (recorder.skip() && (isRecording || isPlayingBack)) {
testContext.skip();
}
// if (recorder.skip() && (isRecording || isPlayingBack)) {
// testContext.skip();
// }

// If neither recording nor playback is enabled, requests hit the live-service and no recordings are generated
if (isRecording) {
Expand All @@ -453,6 +453,26 @@ export function record(testContext: any) {
recorder.stop();
}
},
/**
* @param runtime [string] Let the recorder know when to skip the test - browser or node.js runtime.
*/
skip: function(runtime: string) {
if (isRecording) {
if (runtime === "node" && !isBrowser()) {
recorder.stop();
testContext.skip();
} else if (runtime === "browser" && isBrowser()) {
recorder.stop();
testContext.skip();
}
} else if (isPlayingBack) {
if (runtime === "node" && !isBrowser()) {
testContext.skip();
} else if (runtime === "browser" && isBrowser()) {
testContext.skip();
}
}
},
getUniqueName: function(prefix: string, recorderId?: string): string {
let name: string;
if (!recorderId) {
Expand Down