Added new test cases by at00825957 · Pull Request #813 · openBackhaul/ApplicationPattern

at00825957 · 2023-07-11T15:03:51Z

Fixes #648

DanaSunal

Same comments apply to all three test files.

DanaSunal · 2023-07-17T07:23:59Z

+  },
+  {
+    forwardingName: 'ServiceRequestCausesLtpUpdateRequest',
+    attributeList: {


AttributeList has 0 information, the parameter is passed from automateForwardingConstruct to eventDispatcher without any alternation, it can safely be an empty array.

DanaSunal · 2023-07-17T07:38:17Z

+      '550e8400-e29b-11d4-a716-446655440000', '1.3.1', 'Unknown value');
+    return res.then(() => fail())
+    .catch((err)=> {
+      expect(err).toEqual(mockError);


Also if you put expect only in catch block and no error is caught, the test will again test nothing.

DanaSunal · 2023-07-26T11:37:10Z

Please go through existing comments, a lot of them were not addressed yet.

at00825957 · 2023-07-26T14:10:09Z

Please go through existing comments, a lot of them were not addressed yet.

@DanaSunal made changes for every comment already.

DanaSunal · 2023-07-26T19:13:49Z

+};
+
+describe("configureForwardingConstructAsync", () => {
+  test("automateForwardingConstructAsync - failed to get LogicalTerminationPointListAsync", async () => {


This test and "automateForwardingConstructAsync - failed to get LogicalTerminationPointListAsync when FC Port is subscription" are identical. It does not matter that they have different attributeList, because the mocked error is thrown before the decision "isOperationOfFcPortType" is even made.

DanaSunal · 2023-07-26T19:15:43Z

+        '1.3.1', 'Unknown value');
+
+    return res.then()
+      .catch((err)=> {


This test never reaches the catch block. As I wrote before, if you put expect in catch block and the catch block is not reached, test will pass without ever reaching the expect.

Issue I wrote above is still present.

DanaSunal · 2023-07-26T19:16:41Z

+    })
+  });
+
+  test("automateForwardingConstructWithoutInputAsync - if ForwardingConstructForTheForwardingNameAsync returns undefined when FC Port is subscription", async () => {


Same as "automateForwardingConstructWithoutInputAsync - if ForwardingConstructForTheForwardingNameAsync returns undefined".

DanaSunal · 2023-07-26T19:17:20Z

+    })
+  });
+
+  test("automateForwardingConstructAsync - if LogicalTerminationPointListAsync returns undefined", async () => {


This test fails. Please run the tests before pushing the code.

DanaSunal · 2023-07-26T19:17:34Z

+    })
+  });
+
+  test("automateForwardingConstructAsync - if LogicalTerminationPointListAsync returns undefined when FC Port is subscription", async () => {


This test also fails.

DanaSunal · 2023-08-28T07:43:25Z

+        '1.3.1', 'Unknown value');
+
+    return res.then()
+      .catch((err)=> {


Issue I wrote above is still present.

DanaSunal · 2023-08-28T07:46:24Z

+
+    const res = await ForwardingConstructConfigurationServices.configureForwardingConstructAsync(
+      operationServerName, forwardingConfigurationInputList);
+    expect(res).toBeInstanceOf(ForwardingConstructConfigurationStatus);


Instanceof is not enough. You should expect exact values.

DanaSunal · 2023-08-28T08:01:25Z

@@ -0,0 +1,241 @@
+const ForwardingConstructConfigurationServices = require('./ForwardingConstructConfigurationServices');


This file only tests about half of the class. The tests do not touch methods eg. updateFcPortAsync, deleteFcPortAsync, addFcPortAsync... (run jest command with '--coverage' to see details).

deleteFcPortAsync is still not being tested at all.

DanaSunal · 2023-09-05T07:15:05Z

+        '1.3.1', 'Unknown value');
+
+    return res.then()
+      .catch((err)=> {


Issue I wrote above is still present.

DanaSunal · 2023-09-05T07:29:27Z

+
+    const res = ForwardingConstructConfigurationServices.configureForwardingConstructAsync(
+      operationServerName, forwardingConfigurationInputList);
+    return res.then()


the '.then()' part is useless. Also although it works, this is not the way it should be tested using jest. Take a look at: https://jestjs.io/docs/expect#rejects
test('rejects to octopus', async () => { await expect(Promise.reject(new Error('octopus'))).rejects.toThrow('octopus'); });

DanaSunal · 2023-09-05T07:30:32Z

+    const res = ForwardingConstructConfigurationServices.configureForwardingConstructAsync(
+      operationServerName, forwardingConfigurationInputList);
+    return res.then(() => {
+      return Promise.reject(mockError);


Here the '.then()' part is not only useless but misleading, because you are throwing the error you are expecting, which means the test will ALWAYS pass, no matter what 'res' contains.

DanaSunal · 2023-09-05T07:32:29Z

+    jest.spyOn(HttpClientInterface, 'getReleaseNumberAsync').mockReturnValueOnce('2.0.1');
+    jest.spyOn(LogicalTerminationPoint, 'getServerLtpListAsync').mockReturnValue(["ro-2-0-1-op-c-im-ol-1-0-0-003"]);
+    jest.spyOn(HttpClientInterface, 'getApplicationNameAsync').mockReturnValue('RegistryOffice')
+    jest.spyOn(HttpClientInterface, 'getReleaseNumberAsync').mockReturnValue('2.0.2');


To return different values from the same mocked method use chaining: https://jestjs.io/docs/mock-function-api#mockfnmockreturnvalueoncevalue

DanaSunal · 2023-09-05T07:35:41Z

@@ -0,0 +1,241 @@
+const ForwardingConstructConfigurationServices = require('./ForwardingConstructConfigurationServices');


deleteFcPortAsync is still not being tested at all.

DanaSunal · 2023-09-05T07:36:07Z

 afterEach(() => {
  jest.resetAllMocks();
-});
+});


Do not push changes to this file.

DanaSunal · 2023-09-07T11:07:50Z

+    await expect(Promise.reject(new Error("Cannot read properties of undefined (reading 'fc-port')d"))).rejects.toThrow(
+      "Cannot read properties of undefined (reading 'fc-port')",
+    );
+  });


This test does not test absolutely anything. You could have written expect('true').toBeTruthy() and it would have the same value. Same for the "automateForwardingConstructWithoutInputAsync - if ForwardingConstructForTheForwardingNameAsync returns undefined" and others in the following file, since the "tests" are identical.

Just remove them.

at00825957 and others added 4 commits July 10, 2023 18:55

adding new test cases

d23ee44

update test cases

b9b0f38

change test case

e7b2ed1

Merge branch 'develop' into AmanTyagi/issue648

f1f9740

at00825957 requested review from DanaSunal, IswaryaaS, MartinSunal, PrathibaJee and vanithavalluripalli9 as code owners July 11, 2023 15:03

DanaSunal suggested changes Jul 12, 2023

View reviewed changes

DanaSunal self-assigned this Jul 12, 2023

at00825957 added 2 commits July 17, 2023 11:07

Merge branch 'develop' into AmanTyagi/issue648

368496b

add negative test cases

7087637

DanaSunal suggested changes Jul 17, 2023

View reviewed changes

at00825957 added 2 commits July 18, 2023 14:57

Merge branch 'develop' into AmanTyagi/issue648

b025fcb

resolve comments

a302cbb

Merge branch 'develop' into AmanTyagi/issue648

0f2ca61

DanaSunal suggested changes Jul 26, 2023

View reviewed changes

at00825957 added 3 commits July 28, 2023 13:57

Merge branch 'develop' into AmanTyagi/issue648

ed8f3d9

Merge branch 'develop' into AmanTyagi/issue648

4c5319e

resolve comments

3e988cb

DanaSunal suggested changes Aug 28, 2023

View reviewed changes

at00825957 added 3 commits August 30, 2023 17:36

Resolvs comments

86260bd

resolve test case issue

156c5bb

resolve lint error

870c6de

DanaSunal suggested changes Sep 5, 2023

View reviewed changes

DanaSunal mentioned this pull request Sep 6, 2023

issue-780 : removed global variable #781

Merged

merge

d813734

resolve comments

6dd79c9

DanaSunal suggested changes Sep 7, 2023

View reviewed changes

Remove redundant test

a95e4bc

DanaSunal approved these changes Sep 8, 2023

View reviewed changes

DanaSunal merged commit dc79692 into openBackhaul:develop Sep 8, 2023

		@@ -0,0 +1,241 @@
		const ForwardingConstructConfigurationServices = require('./ForwardingConstructConfigurationServices');

Conversation

at00825957 commented Jul 11, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DanaSunal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanaSunal commented Jul 26, 2023

Uh oh!

at00825957 commented Jul 26, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants