Expand the end-to-end suite from 7 tests to 39 - #78
Open
milanmalhotra wants to merge 3 commits into
Open
Conversation
The harness disabled the delay, cancel-on-move, safety and the max-homes cap so an assertion could never race a timer. That left all four untested. Turning them on costs nothing, because a holder of sh2.bypass-teleport-delay takes a delay of 0 and skips attempt tracking, and a holder of sh2.bypass-max-homes skips the cap. Both sit under sh2.admin. Granting sh2.player to everyone rather than to ops gives two tiers: a bot that ops keeps the instant, uncapped path, and one that does not exercises the real thing. sh2.move-home is pinned to op so a spec can prove PermissionOverrides detaches a node from the bundle it belongs to. The homes spec's permission test goes with it. Its premise was that a non-op holds nothing, which is no longer true; permissions.spec.ts covers that ground against an admin command instead.
Thirty-two more end-to-end tests, taking the suite from 7 to 39 in about twenty seconds. They cover what MockBukkit cannot reach rather than restating the unit suite: real Brigadier permission gating, the teleport countdown and its cancel, inventory clicks from a real client, two players at once, and the nether and the end. Two helpers come with them. dimensions.ts clears and floors a pocket before teleporting, because the generator decides what is at any spot in the nether and teleportSafety refuses a home inside blocks. menus.ts opens a home's management menu, which needs a raw mineflayer right click since the locator API clicks with the left button only.
Records the new test count and the two permission tiers the specs test against, including why a spec that must stand still through the countdown disables the bot's physics first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #76
Takes the end-to-end suite from 7 tests to 39, running in about twenty seconds.
The new cases cover what MockBukkit cannot reach rather than restating the 389
unit tests: real Brigadier permission gating, the teleport countdown and its
cancel, inventory clicks from a real client, two players at once, and the nether
and the end.
It found a bug
The GUI rename prompt crashes on Paper 1.21.11. AnvilGUI's static initializer
looks up
net.minecraft.world.entity.player.EntityHuman, the Spigot mappedname, and Paper serves Mojang mappings, so clicking Rename throws
NoClassDefFoundErrorand the prompt never opens. Filed as #77 with the stacktrace.
The spec that found it is written and works right up to the crash. It is held
out of this pull request so the suite stays green, and goes in with the fix.
The fixture change
The harness disabled the teleport delay, cancel-on-move, safety and the
max-homes cap so an assertion could never race a timer, which left all four
untested. They are on now, and no existing test needed changing, because a
holder of
sh2.bypass-teleport-delaytakes a delay of 0 and a holder ofsh2.bypass-max-homesskips the cap, both undersh2.admin.Granting
sh2.playerto everyone rather than to ops gives the specs two tiers:a bot that calls
makeOp()keeps the instant, uncapped path, and one that doesnot exercises the real behavior.
sh2.move-homeis pinned toopso a spec canprove
PermissionOverridesdetaches a node from its bundle.Coverage
permissions.spec.tsteleport.spec.tsgui.spec.tsadmin.spec.tsblacklist.spec.tsworlds.spec.tsmaxhomes.spec.tsitems.spec.tsTwo helpers:
dimensions.tsclears and floors a pocket before teleporting,since the generator decides what is at any spot in the nether and
teleportSafetyrefuses a home inside blocks.menus.tsopens a home'smanagement menu, which needs a raw mineflayer right click because the locator
API clicks with the left button only.
Left out
Pagination. Forty-six sequential
/create-homecalls risk Paper's chat spamkick, and
HomesGuiPaginationTestalready covers it in process.The homes spec's permission test is gone. Its premise was that a non-op holds
nothing, which the fixture change makes untrue;
permissions.spec.tscoversthat ground against an admin command instead.
Noticed while working, not addressed here
TeleportAttemptsDaocompares the stored location to the current one with exactdouble equality, so any position change at all cancels a pending teleport. An
idle bot sending position packets was enough, which is why specs that stand
still through the countdown set
bot.physicsEnabled = false. On a live serverthat means standing on a boat or a piston would cancel a teleport.
PlayerMoveWhileTeleportingnever setscanTeleport; it only logs. The cancelactually happens in the DAO on each countdown tick, so the listener is querying
SQLite on every move event while doing nothing functional. That may make #46
larger than it reads.
Testing
39 passed, 0 failed, 18.5s.