Skip to content

Commit 23f60d2

Browse files
rom1504claude
andauthored
Improve test reliability: reconnect, retry, server console, packet fix (#3873)
* Fix chatAddPattern test to filter by expected username The test used once(bot, 'chat') which matches ANY chat message. Admin messages from /give or /clear in resetState could arrive and match before the test's tellraw. Now filters for username === 'U9G'. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use server console for clearInventory give and clear Now that chatAddPattern test filters by username, server console admin messages no longer interfere. Server console commands are more reliable than bot.chat — they work even if the bot is in a bad state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Use server console for teleport in resetState Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Reduce resetState operation timeout from 20s to 5s Operations like gamemode change, clear inventory, and teleport complete in <1s. A 5s timeout fails faster and lets retries kick in sooner instead of waiting 20s per failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Run example tests last to limit blast radius Tests that spawn child processes (exampleBee, exampleDigger, exampleInventory) can crash and disconnect the bot, cascading to fail all subsequent tests. Running them last ensures they can only affect each other, not the main test suite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Auto-reconnect bot after disconnect in runTest When a test crashes and disconnects the bot (e.g. exampleDigger child process failure), subsequent tests would cascade-fail. Now detects !bot.entity and reconnects with a fresh bot instance before running the next test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clearInventory: use bot.chat for /give, server console for /clear Server console /give doesn't trigger updateSlot on 1.21.9+. Use bot.chat('/give @A stone 1') for the give (works on all versions) and wrap.writeServer for /clear (more reliable than chat for clear). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add set_player_inventory handler for 1.21.9+ server commands MC 1.21.9+ uses a new set_player_inventory packet for server-initiated inventory changes (e.g. console /give) instead of set_slot. Mineflayer had no handler for it, so updateSlot events never fired. This fixes server console /give on 1.21.9+ and also benefits any user code that relies on inventory updates from server commands. Also reverts clearInventory to use server console for both give and clear now that the packet handler is in place. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Trigger CI with set_player_inventory handler * Fix clearInventory: use bot.chat for /give on all versions Server console /give doesn't send inventory update packets to the client on 1.21.9+ (no set_slot nor set_player_inventory). The item gets added server-side but the client never knows about it. Use bot.chat('/give @A stone 1') which goes through the player's command executor and always sends proper set_slot packets. Keep server console for /clear (which does send set_slot reliably). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase /give timeout to 10s for loaded CI runners The 5s timeout is fine for server console commands but bot.chat goes through the network stack and can be slower on loaded runners with 4 MC servers sharing one container. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clearInventory: use bot.chat for /clear too on 1.21.9+ Server console /clear also doesn't send proper inventory packets on 1.21.9+. Use bot.chat for both /give and /clear, waiting for the chat message confirmation for /clear. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: rom1504 <rom1504@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7af5e2c commit 23f60d2

4 files changed

Lines changed: 54 additions & 10 deletions

File tree

lib/plugins/inventory.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,12 @@ function inject (bot, { hideErrors }) {
728728
const newItem = Item.fromNotch(packet.item)
729729
bot._setSlot(packet.slot, newItem, window)
730730
})
731+
// 1.21.9+ uses set_player_inventory for server-initiated inventory changes
732+
// (e.g. console /give) instead of set_slot
733+
bot._client.on('set_player_inventory', (packet) => {
734+
const newItem = Item.fromNotch(packet.contents)
735+
bot._setSlot(packet.slotId, newItem)
736+
})
731737
bot._client.on('window_items', (packet) => {
732738
const window = packet.windowId === 0 ? bot.inventory : bot.currentWindow
733739
if (!window || window.id !== packet.windowId) {

test/externalTest.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const fs = require('fs')
88
const path = require('path')
99

1010
const { getPort } = require('./common/util')
11+
const { once } = require('../lib/promise_utils')
1112

1213
// set this to false if you want to test without starting a server automatically
1314
const START_THE_SERVER = true
@@ -124,10 +125,39 @@ for (const supportedVersion of mineflayer.testedVersions) {
124125
})
125126
})
126127

128+
async function reconnectBot () {
129+
console.log(' Bot disconnected, reconnecting...')
130+
try { bot.end() } catch (e) { /* ignore */ }
131+
await new Promise(resolve => setTimeout(resolve, 1000))
132+
bot = mineflayer.createBot({
133+
username: 'flatbot',
134+
viewDistance: 'tiny',
135+
port: PORT,
136+
host: '127.0.0.1',
137+
version: supportedVersion
138+
})
139+
commonTest(bot, wrap)
140+
bot.test.port = PORT
141+
await once(bot, 'spawn')
142+
console.log(' Bot reconnected')
143+
wrap.writeServer('op flatbot\n')
144+
await new Promise(resolve => setTimeout(resolve, 2000))
145+
}
146+
127147
const externalTestsFolder = path.resolve(__dirname, './externalTests')
128148
let distinctFailures = 0
149+
// Sort test files so example tests (which spawn child processes and can
150+
// crash/disconnect the bot) run last, limiting their blast radius.
151+
const dangerousTests = ['exampleBee', 'exampleDigger', 'exampleInventory']
129152
fs.readdirSync(externalTestsFolder)
130153
.filter(file => fs.statSync(path.join(externalTestsFolder, file)).isFile())
154+
.sort((a, b) => {
155+
const aName = path.basename(a, '.js')
156+
const bName = path.basename(b, '.js')
157+
const aDangerous = dangerousTests.includes(aName) ? 1 : 0
158+
const bDangerous = dangerousTests.includes(bName) ? 1 : 0
159+
return aDangerous - bDangerous
160+
})
131161
.forEach((test) => {
132162
test = path.basename(test, '.js')
133163
const testFunctions = require(`./externalTests/${test}`)(supportedVersion)
@@ -140,7 +170,11 @@ for (const supportedVersion of mineflayer.testedVersions) {
140170
if (this.test._currentRetry > 0) {
141171
console.log(` [retry ${this.test._currentRetry}] ${testName}`)
142172
}
143-
bot.test.resetState()
173+
// Reconnect if bot got disconnected by a previous test
174+
const reconnect = !bot.entity
175+
? reconnectBot()
176+
: Promise.resolve()
177+
reconnect.then(() => bot.test.resetState())
144178
.then(() => {
145179
bot.test.sayEverywhere(`### Starting ${testName}`)
146180
return testFunction(bot, done)

test/externalTests/chat.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
const assert = require('assert')
2-
const { once } = require('../../lib/promise_utils')
2+
const { once, onceWithCleanup } = require('../../lib/promise_utils')
33

44
module.exports = () => {
55
async function runTest (bot, testFunction) {
@@ -27,7 +27,10 @@ module.exports = () => {
2727
addTest('test chatAddPattern', async (bot) => {
2828
await once(bot, 'message') // => starting chat test chatAddPattern
2929
bot.chat('/tellraw @p {"translate":"chat.type.text", "with":["U9G", "Hello World!"]}')
30-
const [username, message, translate, chatMessage] = await once(bot, 'chat')
30+
const [username, message, translate, chatMessage] = await onceWithCleanup(bot, 'chat', {
31+
timeout: 20000,
32+
checkCondition: (username) => username === 'U9G'
33+
})
3134
assert.strictEqual(username, 'U9G')
3235
assert.strictEqual(message, 'Hello World!')
3336
assert.strictEqual(translate, 'chat.type.text')

test/externalTests/plugins/testCommon.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const process = require('process')
66
const assert = require('assert')
77
const { sleep, onceWithCleanup } = require('../../../lib/promise_utils')
88

9-
const timeout = 20000
9+
const timeout = 5000
1010
module.exports = inject
1111

1212
function inject (bot, wrap) {
@@ -123,15 +123,15 @@ function inject (bot, wrap) {
123123
}
124124

125125
async function clearInventory () {
126-
// Give a stone then clear — same as the original approach.
127-
// The /give ensures the server has something to clear.
126+
// Use bot.chat for /give (server console /give doesn't send inventory
127+
// update packets on 1.21.9+). Use server console for /clear.
128128
bot.chat('/give @a stone 1')
129129
await onceWithCleanup(bot.inventory, 'updateSlot', {
130-
timeout,
130+
timeout: 10000,
131131
checkCondition: (slot, oldItem, newItem) => newItem?.name === 'stone'
132132
})
133133
const clearMsg = onceWithCleanup(bot, 'message', {
134-
timeout,
134+
timeout: 10000,
135135
checkCondition: msg => msg.translate === 'commands.clear.success.single' || msg.translate === 'commands.clear.success'
136136
})
137137
bot.chat('/clear')
@@ -145,10 +145,11 @@ function inject (bot, wrap) {
145145
}
146146

147147
async function teleport (position) {
148+
// Use server console for teleport — works even if bot is in a bad state
148149
if (bot.supportFeature('hasExecuteCommand')) {
149-
bot.test.sayEverywhere(`/execute in overworld run teleport ${bot.username} ${position.x} ${position.y} ${position.z}`)
150+
wrap.writeServer(`execute in overworld run teleport ${bot.username} ${position.x} ${position.y} ${position.z}\n`)
150151
} else {
151-
bot.test.sayEverywhere(`/tp ${bot.username} ${position.x} ${position.y} ${position.z}`)
152+
wrap.writeServer(`tp ${bot.username} ${position.x} ${position.y} ${position.z}\n`)
152153
}
153154
return onceWithCleanup(bot, 'move', {
154155
timeout,

0 commit comments

Comments
 (0)