Skip to content

test: give the declare_commands tree the packet's rootIndex - #1527

Open
u9g wants to merge 1 commit into
PrismarineJS:masterfrom
u9g:fix/declare-commands-root-index
Open

test: give the declare_commands tree the packet's rootIndex#1527
u9g wants to merge 1 commit into
PrismarineJS:masterfrom
u9g:fix/declare-commands-root-index

Conversation

@u9g

@u9g u9g commented Sep 7, 2026

Copy link
Copy Markdown
Member

The declare_commands packet carries nodes and rootIndex. The tree this test emits has no rootIndex, so the client reads nodes[undefined], rejects the tree and ends the connection, and the assertion never runs.

Invariant: a declare_commands packet built by a test names its root, so the client accepts the tree.

The test has failed since it was added. CI does not report it: mochaTest is invoked with -g <version>v and no name in this file carries a Minecraft version, so the file never runs. #1526 removes that filter.

The tree the test emits has no rootIndex, so the client reads
nodes[undefined], rejects the tree and ends the connection, and the
test's only assertion never runs. It has failed since it was added.
Nothing reported it because CI runs mochaTest with a -g filter on the
Minecraft version and no name in this file carries one.
u9g added a commit to u9g/node-minecraft-protocol that referenced this pull request Sep 7, 2026
…e grep

CI ran mochaTest with -g <version>v, so a suite only ran if its title
carried a Minecraft version. The three suites that are not
version-parameterised, declare_commands handling, lpVec3 and ping, never
ran in any job.

The version loops now read the list from test/common/testedVersions,
which honours MC_VERSION and rejects a value that is not a supported
version, so CI selects the version without touching test titles and
every suite runs in every job. Running mocha with no MC_VERSION still
tests every supported version.

declare_commands handling fails until PrismarineJS#1527 lands, which is the tree
fix this filter was hiding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant