Skip to content

Commit 035ce9f

Browse files
authored
Fix: correct bugs in test files (#1418)
### Summary Fixed 6 bugs across 6 test files discovered during code review. All issues were in test code only — no library changes. ### Changes #### 1. `test/client_setup.js` — Remove reference to undefined `timer` The `SIGINT` handler called `timer.cancel()`, but `timer` is not defined in this scope. Removed the call to prevent a `ReferenceError` if the signal is received. #### 2. `test/test-lifecycle.js` — Fix duplicate transition index Two consecutive assertions both checked `transitions[1]`, meaning `transitions[2]` was never validated. Changed the second check from `transitions[1]` to `transitions[2]`. #### 3. `test/test-parameter-service.js` — Fix wrong parameter name `Parameter.fromParameterMessage` was called with `name: 'p1'` but the test was validating parameter `A.p3`. Changed to `name: 'A.p3'`. #### 4. `test/test-rate.js` — Add missing assertion The test computed an average sleep time but never asserted on it. Added an assertion that the average is within 5x the expected period. #### 5. `test/test-utils.js` — Remove empty duplicate test An empty test body `it('should valid ensureDirSync works correctly', function () {})` duplicated the immediately following test `it('should verify ensureDirSync works correctly', ...)`. Removed the empty duplicate. #### 6. `test/types/index.test-d.ts` — Remove misplaced type assertion `expectType<boolean>(client.isServiceServerAvailable())` was in the **ActionClient** section but tested `client` (the service client variable). The same assertion already exists in the correct Client section. Removed the misplaced copy. ### Files changed | File | Change | |------|--------| | `test/client_setup.js` | Remove undefined `timer.cancel()` | | `test/test-lifecycle.js` | `transitions[1]` → `transitions[2]` in second assertion | | `test/test-parameter-service.js` | `'p1'` → `'A.p3'` | | `test/test-rate.js` | Add avg sleep time assertion | | `test/test-utils.js` | Remove empty duplicate test | | `test/types/index.test-d.ts` | Remove misplaced `client.isServiceServerAvailable()` | Fix: #1417
1 parent ba6f9a5 commit 035ce9f

6 files changed

Lines changed: 11 additions & 8 deletions

File tree

test/client_setup.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ rclnodejs
3737
rclnodejs.spin(node);
3838

3939
process.on('SIGINT', function () {
40-
timer.cancel();
4140
node.destroy();
4241
rclnodejs.shutdown();
4342
process.exit(0);

test/test-lifecycle.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,9 @@ describe('LifecycleNode test suite', function () {
260260
transitions[1].transition.id === 6
261261
); // shutdown
262262
assert.ok(
263-
transitions[1].transition.id === 2 || // cleanup
264-
transitions[1].transition.id === 3 || // activate
265-
transitions[1].transition.id === 6
263+
transitions[2].transition.id === 2 || // cleanup
264+
transitions[2].transition.id === 3 || // activate
265+
transitions[2].transition.id === 6
266266
); // shutdown
267267
});
268268

test/test-parameter-service.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ describe('Parameter_server tests', function () {
251251

252252
// A.p3 value
253253
const p3 = Parameter.fromParameterMessage({
254-
name: 'p1',
254+
name: 'A.p3',
255255
value: response.values[1],
256256
});
257257
assert.equal(p3.type, ParameterType.PARAMETER_BOOL);

test/test-rate.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,5 +107,12 @@ describe('rclnodejs rate test suite', function () {
107107
arr.reduce((prev, cur) => {
108108
return prev + cur;
109109
}, 0) / dataSize;
110+
111+
// avg sleep time should be within a reasonable range of the expected period
112+
const expectedPeriod = 1000 / hz; // 1ms
113+
assert.ok(
114+
avg < expectedPeriod * 5,
115+
`Average sleep time ${avg}ms exceeded 5x the expected period ${expectedPeriod}ms`
116+
);
110117
});
111118
});

test/test-utils.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ describe('Utils testing', function () {
4545
await utils.ensureDir(dir);
4646
});
4747

48-
it('should valid ensureDirSync works correctly', function () {});
49-
5048
it('should verify ensureDirSync works correctly', function () {
5149
const dir = path.join(tmpDir, 'nested/sync/dir');
5250
utils.ensureDirSync(dir);

test/types/index.test-d.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,6 @@ const actionClient = new rclnodejs.ActionClient(
472472
expectType<rclnodejs.ActionClient<'example_interfaces/action/Fibonacci'>>(
473473
actionClient
474474
);
475-
expectType<boolean>(client.isServiceServerAvailable());
476475
expectType<Promise<boolean>>(actionClient.waitForServer());
477476
expectType<void>(actionClient.destroy());
478477

0 commit comments

Comments
 (0)