Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/linux-arm64-build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ jobs:
with:
required-ros-distributions: ${{ matrix.ros_distribution }}

- name: Install test-msgs on Linux
- name: Install test-msgs and mrpt_msgs on Linux
run: |
sudo apt install ros-${{ matrix.ros_distribution }}-test-msgs
sudo apt install ros-${{ matrix.ros_distribution }}-test-msgs ros-${{ matrix.ros_distribution }}-mrpt-msgs
# Adjust dependencies based on Ubuntu version
LIBASOUND_PKG="libasound2"
if grep -q "24.04" /etc/os-release; then
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/linux-x64-build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ jobs:
required-ros-distributions: ${{ matrix.ros_distribution }}
use-ros2-testing: ${{ matrix.ros_distribution == 'rolling' }}

- name: Install test-msgs on Linux
- name: Install test-msgs and mrpt_msgs on Linux
run: |
sudo apt install ros-${{ matrix.ros_distribution }}-test-msgs
sudo apt install ros-${{ matrix.ros_distribution }}-test-msgs ros-${{ matrix.ros_distribution }}-mrpt-msgs
# Adjust dependencies based on Ubuntu version
LIBASOUND_PKG="libasound2"
if grep -q "24.04" /etc/os-release; then
Expand Down
7 changes: 6 additions & 1 deletion rosidl_gen/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,12 @@ function getSubFolder(filePath, amentExecuted) {
}

if (amentExecuted) {
return filePath.match(/\w+\/share\/\w+\/(\w+)\//)[1];
const match = filePath.match(/\w+\/share\/\w+\/([\w-]+)\//);
if (match) {
// Handle non-standard subfolder names (e.g., msg-common, msg-ros2)
// by extracting only the base interface type before any hyphen.
return match[1].split('-')[0];
}
}
// If the |amentExecuted| equals to false, the file's extension will be assigned as
Comment on lines 61 to 62
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

When the regex match fails (match is null or undefined), the function will fall through to line 64 which returns the file extension. However, if amentExecuted is true and the match fails, this might indicate a malformed path that should be handled differently. Consider adding an explicit else clause or a fallback return statement within the amentExecuted block to make the error handling more explicit, or add a comment explaining that falling through to line 64 is intentional for this case.

Suggested change
}
// If the |amentExecuted| equals to false, the file's extension will be assigned as
// If |amentExecuted| is true but the expected pattern is not found,
// fall back to using the file's extension as the subfolder name.
return path.parse(filePath).ext.substr(1);
}
// If |amentExecuted| equals to false, the file's extension will be assigned as

Copilot uses AI. Check for mistakes.
// the name of sub folder.
Expand Down
5 changes: 2 additions & 3 deletions scripts/run_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ utils
});

mocha.run(function (failures) {
process.on('exit', () => {
process.exit(failures);
});
process.exitCode = failures ? 1 : 0;
process.exit(process.exitCode);
});
})
.catch((err) => {
Expand Down
46 changes: 23 additions & 23 deletions test/test-native-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,35 @@ const sinon = require('sinon');
const fs = require('fs');
const child_process = require('child_process');

// Require the module once without clearing cache to avoid triggering
// loadNativeAddon() repeatedly. The TestHelpers functions read process
// state at call time, so they can be tested with platform/env changes
// without re-requiring the module.
// NOTE: The module may have been loaded by other tests before NODE_ENV was set,
// so TestHelpers might not be present. We clear cache and re-require with
// NODE_ENV='test' to ensure TestHelpers is exported.
const nativeLoaderPath = require.resolve('../lib/native_loader.js');
if (
!require.cache[nativeLoaderPath] ||
!require.cache[nativeLoaderPath].exports.TestHelpers
) {
delete require.cache[nativeLoaderPath];
process.env.NODE_ENV = 'test';
}
const nativeLoader = require('../lib/native_loader.js');

describe('NativeLoader testing', function () {
const sandbox = sinon.createSandbox();
let originalPlatform;
let originalArch;
let originalEnv;
let loader;

beforeEach(function () {
originalPlatform = process.platform;
originalArch = process.arch;
originalEnv = { ...process.env };
process.env.NODE_ENV = 'test';

// Clear cache to reload module
delete require.cache[require.resolve('../lib/native_loader.js')];
loader = nativeLoader.TestHelpers;
});

afterEach(function () {
Expand All @@ -29,20 +44,14 @@ describe('NativeLoader testing', function () {
process.env = originalEnv;
});

function getLoader() {
return require('../lib/native_loader.js').TestHelpers;
}

it('customFallbackLoader returns null on non-linux', function () {
Object.defineProperty(process, 'platform', { value: 'win32' });
const loader = getLoader();
assert.strictEqual(loader.customFallbackLoader(), null);
});

it('customFallbackLoader returns null if env info missing', function () {
Object.defineProperty(process, 'platform', { value: 'linux' });
process.env.ROS_DISTRO = '';
const loader = getLoader();
assert.strictEqual(loader.customFallbackLoader(), null);
});

Expand All @@ -52,8 +61,6 @@ describe('NativeLoader testing', function () {
process.env.ROS_DISTRO = 'humble';

sandbox.stub(fs, 'existsSync').returns(false);

const loader = getLoader();
assert.strictEqual(loader.customFallbackLoader(), null);
});

Expand All @@ -64,8 +71,6 @@ describe('NativeLoader testing', function () {

// Stub fs.existsSync to return true
const existsSync = sandbox.stub(fs, 'existsSync').returns(true);

const loader = getLoader();
assert.strictEqual(loader.customFallbackLoader(), null);

// Verify it checked for the file
Expand All @@ -79,17 +84,12 @@ describe('NativeLoader testing', function () {
process.env.RCLNODEJS_FORCE_BUILD = '1';
const execSync = sandbox.stub(child_process, 'execSync');

// We expect it to try loading bindings after build
// Since we don't mock bindings, it will load the real one (if present) or fail.
// If it loads real one, test passes.
// If it fails, loadNativeAddon throws. We might need to handle that.
// But usually in dev env, the binding exists.
// Clear cache and re-require to trigger loadNativeAddon with RCLNODEJS_FORCE_BUILD.
// execSync is stubbed so no real rebuild (which deletes generated/) occurs.
delete require.cache[require.resolve('../lib/native_loader.js')];

try {
getLoader();
// Wait, getLoader requires the file.
// The file calls loadNativeAddon() immediately.
// So valid test is just requiring the file.
require('../lib/native_loader.js');
} catch (e) {
// Ignore if loading binding fails, as long as execSync was called
}
Expand Down
42 changes: 42 additions & 0 deletions test/test-rosidl-message-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,4 +308,46 @@ describe('ROSIDL Node.js message generator test suite', function () {
process.env.AMENT_PREFIX_PATH = amentPrefixPathOriginal;
}
});

it('Testing mrpt_msgs/msg/GraphSlamAgents from non-standard msg subfolder', function () {
// GraphSlamAgents.msg lives under msg-common/ (non-standard subfolder name)
// and references GraphSlamAgent.msg from msg-ros2/. This verifies that
// packages with hyphenated subfolder names are generated and loadable.
const GraphSlamAgents = rclnodejs.require('mrpt_msgs/msg/GraphSlamAgents');
const GraphSlamAgent = rclnodejs.require('mrpt_msgs/msg/GraphSlamAgent');

// Verify GraphSlamAgents can be instantiated with an empty list
const agents = new GraphSlamAgents();
assert.ok(agents.list);
assert.equal(agents.list.size, 0);

// Verify GraphSlamAgent fields and defaults
const agent = new GraphSlamAgent();
assert.equal(agent.name.data, '');
assert.equal(agent.hostname.data, '');
assert.equal(agent.ip_addr.data, '');
assert.equal(agent.port, 0);
assert.equal(agent.is_online.data, false);
assert.equal(agent.last_seen_time.sec, 0);
assert.equal(agent.last_seen_time.nanosec, 0);
assert.equal(agent.topic_namespace.data, '');
assert.equal(agent.agent_id, 0);

// Verify field assignment
agent.name.data = 'robot_1';
agent.hostname.data = 'host1';
agent.ip_addr.data = '192.168.1.17';
agent.port = 11311;
agent.is_online.data = true;
agent.topic_namespace.data = 'robot_1';
agent.agent_id = 17;

assert.equal(agent.name.data, 'robot_1');
assert.equal(agent.hostname.data, 'host1');
assert.equal(agent.ip_addr.data, '192.168.1.17');
assert.equal(agent.port, 11311);
assert.equal(agent.is_online.data, true);
assert.equal(agent.topic_namespace.data, 'robot_1');
assert.equal(agent.agent_id, 17);
});
});
Loading