Skip to content

Clean up the observer tests for better readability #2157

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

Merged
merged 11 commits into from
Apr 5, 2018
Merged

Clean up the observer tests for better readability #2157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bef488e
CsharpLoggerObserver
akshita31 Apr 4, 2018
e38c92d
dotnetchannelobserver
akshita31 Apr 4, 2018
2bd2b31
TelemetryObserver
akshita31 Apr 5, 2018
b298d12
DebugModeObserver
akshita31 Apr 5, 2018
fccf113
channel observer
akshita31 Apr 5, 2018
c5f3cac
Logger Observer
akshita31 Apr 5, 2018
c5e365f
Clean up
akshita31 Apr 5, 2018
5c1a28f
Server error test
akshita31 Apr 5, 2018
d1f077e
Correct message in the debug observer
akshita31 Apr 5, 2018
a1eb7b8
Update
akshita31 Apr 5, 2018
d36f45b
Corrected spelling
akshita31 Apr 5, 2018
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 test/unitTests/logging/CsharpChannelObserver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ suite("CsharpChannelObserver", () => {
new DebuggerPrerequisiteFailure("some failure"),
new ProjectJsonDeprecatedWarning()
].forEach((event: BaseEvent) => {
test(`Shows the channel for ${event.constructor.name}`, () => {
test(`${event.constructor.name}: Channel is shown`, () => {
let hasShown = false;

let observer = new CsharpChannelObserver({
Expand Down
157 changes: 78 additions & 79 deletions test/unitTests/logging/CsharpLoggerObserver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,64 +10,9 @@ import { PlatformInformation } from '../../../src/platform';
import { PackageError } from '../../../src/packages';
import * as Event from '../../../src/omnisharp/loggingEvents';

suite("CsharpLoggerObserver: Download Messages", () => {
suite("CsharpLoggerObserver", () => {
suiteSetup(() => should());

[
{
events: [],
expected: ""
},
{
events: [new Event.DownloadStart("Started")],
expected: "Started"
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100)],
expected: "Started...................."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10), new Event.DownloadProgress(50), new Event.DownloadProgress(100)],
expected: "Started...................."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10), new Event.DownloadProgress(50)],
expected: "Started.........."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50)],
expected: "Started.........."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50), new Event.DownloadProgress(50), new Event.DownloadProgress(50)],
expected: "Started.........."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100), new Event.DownloadSuccess("Done")],
expected: "Started....................Done\n"
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50), new Event.DownloadFailure("Failed")],
expected: "Started..........Failed\n"
},
].forEach((element) => {
test(`Prints the download status to the logger as ${element.expected}`, () => {
let logOutput = "";

let observer = new CsharpLoggerObserver({
...getNullChannel(),
appendLine: (text?: string) => { logOutput += `${text}\n`; },
append: (text?: string) => { logOutput += text; }
});

element.events.forEach((message: Event.BaseEvent) => observer.post(message));
expect(logOutput).to.be.equal(element.expected);
});
});
});

suite('CsharpLoggerObsever', () => {
suiteSetup(() => should());
let logOutput = "";
let observer = new CsharpLoggerObserver({
...getNullChannel(),
Expand All @@ -85,28 +30,83 @@ suite('CsharpLoggerObsever', () => {
expect(logOutput).to.contain("MyArchitecture");
});

test('Event.InstallationFailure: Stage and Error is logged if not a PackageError', () => {
let event = new Event.InstallationFailure("someStage", new Error("someError"));
observer.post(event);
expect(logOutput).to.contain(event.stage);
expect(logOutput).to.contain(event.error.toString());
});

test('Event.InstallationFailure: Stage and Error is logged if a PackageError without inner error', () => {
let event = new Event.InstallationFailure("someStage", new PackageError("someError", null, null));
observer.post(event);
expect(logOutput).to.contain(event.stage);
expect(logOutput).to.contain(event.error.message);
suite("InstallationFailure", () => {
test('Stage and Error is logged if not a PackageError', () => {
let event = new Event.InstallationFailure("someStage", new Error("someError"));
observer.post(event);
expect(logOutput).to.contain(event.stage);
expect(logOutput).to.contain(event.error.toString());
});

test('Stage and Error is logged if a PackageError without inner error', () => {
let event = new Event.InstallationFailure("someStage", new PackageError("someError", null, null));
observer.post(event);
expect(logOutput).to.contain(event.stage);
expect(logOutput).to.contain(event.error.message);
});

test('Stage and Inner error is logged if a PackageError without inner error', () => {
let event = new Event.InstallationFailure("someStage", new PackageError("someError", null, "innerError"));
observer.post(event);
expect(logOutput).to.contain(event.stage);
expect(logOutput).to.contain(event.error.innerError.toString());
});
});

test('Event.InstallationFailure: Stage and Inner error is logged if a PackageError without inner error', () => {
let event = new Event.InstallationFailure("someStage", new PackageError("someError", null, "innerError"));
observer.post(event);
expect(logOutput).to.contain(event.stage);
expect(logOutput).to.contain(event.error.innerError.toString());
suite('Download',() => {
[
{
events: [],
expected: ""
},
{
events: [new Event.DownloadStart("Started")],
expected: "Started"
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100)],
expected: "Started...................."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10), new Event.DownloadProgress(50), new Event.DownloadProgress(100)],
expected: "Started...................."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(10), new Event.DownloadProgress(50)],
expected: "Started.........."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50)],
expected: "Started.........."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50), new Event.DownloadProgress(50), new Event.DownloadProgress(50)],
expected: "Started.........."
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(100), new Event.DownloadSuccess("Done")],
expected: "Started....................Done\n"
},
{
events: [new Event.DownloadStart("Started"), new Event.DownloadProgress(50), new Event.DownloadFailure("Failed")],
expected: "Started..........Failed\n"
},
].forEach((element) => {
test(`Prints the download status to the logger as ${element.expected}`, () => {
let logOutput = "";

let observer = new CsharpLoggerObserver({
...getNullChannel(),
appendLine: (text?: string) => { logOutput += `${text}\n`; },
append: (text?: string) => { logOutput += text; }
});

element.events.forEach((message: Event.BaseEvent) => observer.post(message));
expect(logOutput).to.be.equal(element.expected);
});
});
});


[
{
message: new Event.DebuggerPrerequisiteFailure('Some failure message'),
Expand All @@ -122,25 +122,25 @@ suite('CsharpLoggerObsever', () => {
expect(logOutput).to.contain(element.expected);
}));

test(`Event.ActivaltionFailure: Some message is logged`, () => {
test(`ActivationFailure: Some message is logged`, () => {
let event = new Event.ActivationFailure();
observer.post(event);
expect(logOutput).to.not.be.empty;
});

test(`Event.ProjectJsonDeprecatedWarning: Some message is logged`, () => {
test(`ProjectJsonDeprecatedWarning: Some message is logged`, () => {
let event = new Event.ProjectJsonDeprecatedWarning();
observer.post(event);
expect(logOutput).to.not.be.empty;
});

test(`Event.ProjectJsonDeprecatedWarning: Some message is logged`, () => {
test(`InstallationSuccess: Some message is logged`, () => {
let event = new Event.InstallationSuccess();
observer.post(event);
expect(logOutput).to.not.be.empty;
});

test(`Event.InstallationProgress: Progress message is logged`, () => {
test(`InstallationProgress: Progress message is logged`, () => {
let event = new Event.InstallationProgress("someStage", "someMessage");
observer.post(event);
expect(logOutput).to.contain(event.message);
Expand All @@ -151,5 +151,4 @@ suite('CsharpLoggerObsever', () => {
observer.post(event);
expect(logOutput).to.contain(event.packageInfo);
});

});
40 changes: 15 additions & 25 deletions test/unitTests/logging/DotnetChannelObserver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,24 @@ import { CommandDotNetRestoreStart, BaseEvent } from '../../../src/omnisharp/log

suite("DotnetChannelObserver", () => {
suiteSetup(() => should());
let hasShown: boolean;
let hasCleared: boolean;

[
new CommandDotNetRestoreStart()
].forEach((event: BaseEvent) => {
test(`Clears the channel for ${event.constructor.name}`, () => {
let hasCleared = false;
let observer = new DotNetChannelObserver({
...getNullChannel(),
clear: () => { hasCleared = true; }
});

observer.post(event);
expect(hasCleared).to.be.true;
});
let observer = new DotNetChannelObserver({
...getNullChannel(),
clear: () => { hasCleared = true; },
show: () => { hasShown = true; }
});

[
new CommandDotNetRestoreStart()
].forEach((event: BaseEvent) => {
test(`Shows the channel for ${event.constructor.name}`, () => {
let hasShown = false;
let observer = new DotNetChannelObserver({
...getNullChannel(),
show: () => { hasShown = true; }
});
setup(() => {
hasShown = false;
hasCleared = false;
});

observer.post(event);
expect(hasShown).to.be.true;
});
test(`CommandDotNetRestoreStart : Clears and shows the channel`, () => {
let event = new CommandDotNetRestoreStart();
observer.post(event);
expect(hasCleared).to.be.true;
expect(hasShown).to.be.true;
});
});
70 changes: 37 additions & 33 deletions test/unitTests/logging/OmnisharpDebugModeLoggerObserver.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { OmnisharpDebugModeLoggerObserver } from '../../../src/observers/Omnisha

use(require("chai-string"));

suite("OmnisharpLoggerObserver", () => {
suite("OmnisharpDebugModeLoggerObserver", () => {
suiteSetup(() => should());
let logOutput = "";
let observer = new OmnisharpDebugModeLoggerObserver({
Expand All @@ -30,19 +30,6 @@ suite("OmnisharpLoggerObserver", () => {
});
});

test(`OmnisharpRequestMessage: Request Command and Id is logged`, () => {
let event = new OmnisharpRequestMessage({ command: "someCommand", onSuccess: () => { }, onError: () => { } }, 1);
observer.post(event);
expect(logOutput).to.contain(event.id);
expect(logOutput).to.contain(event.request.command);
});

test(`OmnisharpRequestMessage: Request Data is logged when it is not empty`, () => {
let event = new OmnisharpRequestMessage({ command: "someCommand", onSuccess: () => { }, onError: () => { }, data: "someData" }, 1);
observer.post(event);
expect(logOutput).to.contain(event.request.data);
});

test(`OmnisharpServerEnqueueRequest: Name and Command is logged`, () => {
let event = new OmnisharpServerEnqueueRequest("foo", "someCommand");
observer.post(event);
Expand All @@ -64,36 +51,53 @@ suite("OmnisharpLoggerObserver", () => {
expect(logOutput).to.contain(event.name);
});

test(`OmnisharpEventPacketReceived: Information messages with name OmniSharp.Middleware.LoggingMiddleware and follow pattern /^\/[\/\w]+: 200 \d+ms/ are not logged`, () => {
let event = new OmnisharpEventPacketReceived("INFORMATION", "OmniSharp.Middleware.LoggingMiddleware", "/codecheck: 200 339ms");
observer.post(event);
expect(logOutput).to.contain(event.message);
expect(logOutput).to.contain(event.name);
});

test(`OmnisharpServer messages increase and decrease indent`, () => {
observer.post(new OmnisharpServerVerboseMessage("!indented_1"));
observer.post(new OmnisharpServerProcessRequestStart("name"));
observer.post(new OmnisharpServerVerboseMessage("indented"));
observer.post(new OmnisharpServerProcessRequestComplete());
observer.post(new OmnisharpServerVerboseMessage("!indented_2"));

expect(logOutput).to.startWith(" !indented_1");
expect(logOutput).to.contain("\n indented");
expect(logOutput).to.contain("\n !indented_2");
});

[
new OmnisharpEventPacketReceived("TRACE", "foo", "someMessage"),
new OmnisharpEventPacketReceived("DEBUG", "foo", "someMessage"),
new OmnisharpEventPacketReceived("INFORMATION", "foo", "someMessage"),
new OmnisharpEventPacketReceived("WARNING", "foo", "someMessage"),
new OmnisharpEventPacketReceived("ERROR", "foo", "someMessage"),
new OmnisharpEventPacketReceived("CRITICAL", "foo", "someMessage"),
].forEach((event: OmnisharpEventPacketReceived) => {
test(`OmnisharpEventPacketReceived: ${event.logLevel} messages are logged with name and the message`, () => {
suite('OmnisharpEventPacketReceived', () => {
test(`Information messages with name OmniSharp.Middleware.LoggingMiddleware and follow pattern /^\/[\/\w]+: 200 \d+ms/ are logged`, () => {
Copy link

@rchande rchande Apr 5, 2018

Choose a reason for hiding this comment

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

Regex makes us sad, can you just provide an example?

Copy link
Contributor Author

@akshita31 akshita31 Apr 5, 2018

Choose a reason for hiding this comment

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

The example is there inside the test, so I thought it will be better if we can show the exact pattern here.

Copy link

@rchande rchande Apr 5, 2018

Choose a reason for hiding this comment

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

Ok

let event = new OmnisharpEventPacketReceived("INFORMATION", "OmniSharp.Middleware.LoggingMiddleware", "/codecheck: 200 339ms");
observer.post(event);
expect(logOutput).to.contain(event.message);
expect(logOutput).to.contain(event.name);
});

[
new OmnisharpEventPacketReceived("TRACE", "foo", "someMessage"),
new OmnisharpEventPacketReceived("DEBUG", "foo", "someMessage"),
new OmnisharpEventPacketReceived("INFORMATION", "foo", "someMessage"),
new OmnisharpEventPacketReceived("WARNING", "foo", "someMessage"),
new OmnisharpEventPacketReceived("ERROR", "foo", "someMessage"),
new OmnisharpEventPacketReceived("CRITICAL", "foo", "someMessage"),
].forEach((event: OmnisharpEventPacketReceived) => {
test(`${event.logLevel} messages are not logged`, () => {
observer.post(event);
expect(logOutput).to.be.empty;
});
});
});

suite('OmnisharpRequestMessage', () => {
test(`Request Command and Id is logged`, () => {
let event = new OmnisharpRequestMessage({ command: "someCommand", onSuccess: () => { }, onError: () => { } }, 1);
observer.post(event);
expect(logOutput).to.contain(event.id);
expect(logOutput).to.contain(event.request.command);
});

test(`Request Data is logged when it is not empty`, () => {
let event = new OmnisharpRequestMessage({ command: "someCommand", onSuccess: () => { }, onError: () => { }, data: "someData" }, 1);
observer.post(event);
expect(logOutput).to.be.empty;
expect(logOutput).to.contain(event.request.data);
});
});
});
});
Loading