diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index f0837af5..0e36f13d 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -351,10 +351,19 @@ jobs: QUERY='tests(//tests/...)' # 2) Exclude these tags by default for MacOS. + # + # A test carries `platform-independent` when it exercises no + # behavior that could differ between Linux and MacOS, or when + # whatever platform-dependent behavior it does exercise is also + # exercised by a `medium`-or-smaller test that still runs here. + # Such a test spends time on our slowest and most expensive + # runners for signal that the Linux jobs — which keep running + # every test — already give us. QUERY="$QUERY \ except attr(\"tags\",\"requires-docker\",//tests/...) \ except attr(\"tags\",\"macos_not_supported\",//tests/...) \ except attr(\"tags\",\"requires-linux-x86\",//tests/...) \ + except attr(\"tags\",\"platform-independent\",//tests/...) \ except attr(\"tags\",\"manual\",//tests/...)" # 3) If this is a PR labelled "reboot-release", also drop flaky diff --git a/reboot/aio/state_managers.py b/reboot/aio/state_managers.py index e6fdff59..e7fa2b0c 100644 --- a/reboot/aio/state_managers.py +++ b/reboot/aio/state_managers.py @@ -1547,6 +1547,12 @@ class Lock: transaction. Passing `deadline=None` means wait forever or until the task is cancelled. + - A waiter that has given up, i.e., whose `deadline` elapsed or + whose task was cancelled, is dropped rather than granted (see + `_Waiter.abandoned()`). Granting to one would leave the lock + held by nobody, and every later acquire on the state it guards + would then fail or block forever. + The lock is not `asyncio.Task` aware because it is meant to be used across multiple different calls on the same state, i.e., across transactions. Instead, it tracks shared as a count and @@ -1567,6 +1573,21 @@ def __init__(self, mode: Lock.Mode) -> None: asyncio.get_event_loop().create_future() ) + def abandoned(self) -> bool: + """`True` once this waiter can no longer take the hold it + is queued for, because its acquire deadline elapsed or its + task was cancelled. It is still on the queue only because + it has not been resumed yet to remove itself. Granting to + it would mark the lock held with nobody left to release + it, leaving the state this lock guards unusable for the + life of the process, so such a waiter must be dropped. + + A waiter that is granted is removed from the queue at the + same time its future is resolved, and nothing else + resolves a queued waiter's future, so a waiter that is + still queued and whose future is resolved has given up.""" + return self.future.done() + def __init__(self) -> None: # Number of shared holders. self._shared: int = 0 @@ -1707,6 +1728,17 @@ def _remove_waiter(self, waiter: Lock._Waiter) -> None: except ValueError: pass + def _remove_upgrader(self, waiter: Lock._Waiter) -> None: + """Empties the upgrade slot, but only while `waiter` is the + waiter occupying it. A waiter that has given up may already + have been dropped from the slot, and by the time it cleans up + after itself another holder may have registered an upgrade of + its own there. That one must be left in place: emptying the + slot would leave it registered nowhere, and a waiter nothing + holds a reference to is a waiter nothing can ever grant.""" + if self._upgrader is waiter: + self._upgrader = None + @overload async def _wait( self, @@ -1765,7 +1797,7 @@ async def _wait( await asyncio.wait_for(waiter.future, timeout=timeout) except asyncio.TimeoutError: if upgrade: - self._upgrader = None + self._remove_upgrader(waiter) else: self._remove_waiter(waiter) raise SystemAborted( @@ -1783,12 +1815,18 @@ async def _wait( waiter.future.done() and not waiter.future.cancelled() and waiter.future.exception() is None ): - if mode == Lock.Mode.SHARED: + if upgrade: + # An upgrade whose caller is interrupted leaves + # that caller holding the shared hold it upgrades + # from, so hand that back rather than releasing + # the exclusive hold outright. + self.downgrade() + elif mode == Lock.Mode.SHARED: self.release_shared() else: self.release_exclusive() elif upgrade: - self._upgrader = None + self._remove_upgrader(waiter) else: self._remove_waiter(waiter) raise @@ -1811,15 +1849,21 @@ def _maybe_grant_next(self, *, released: Lock.Mode) -> None: # other remaining shared holders, bypassing any queued # exclusive waiters. assert released == Lock.Mode.SHARED - if self._shared == 1: + if self._upgrader.abandoned(): + # The upgrader has given up, so forget it and grant as + # if it had never asked. It keeps the shared hold it + # upgrades from and releases that itself. + self._upgrader = None + elif self._shared == 1: upgrader = self._upgrader self._upgrader = None self._shared = 0 self._exclusive = True - if not upgrader.future.done(): - upgrader.future.set_result(None) - # While we have an upgrader nothing else gets granted. - return + upgrader.future.set_result(None) + return + else: + # While we have an upgrader nothing else gets granted. + return # Nothing to do if we released a shared hold but still have # shared holders. @@ -1833,6 +1877,11 @@ def _maybe_grant_next(self, *, released: Lock.Mode) -> None: # Grant waiters in FIFO ordering. while self._waiters: waiter = self._waiters[0] + if waiter.abandoned(): + # Drop the waiter and try the next one rather than + # taking the lock on its behalf (see `abandoned()`). + self._waiters.pop(0) + continue if waiter.mode == Lock.Mode.EXCLUSIVE: # We may have granted a shared waiter below if the # _first_ waiter was not exclusive and thus we've now @@ -1841,14 +1890,12 @@ def _maybe_grant_next(self, *, released: Lock.Mode) -> None: return self._waiters.pop(0) self._exclusive = True - if not waiter.future.done(): - waiter.future.set_result(None) + waiter.future.set_result(None) return # The mode is `Lock.Mode.SHARED`. self._waiters.pop(0) self._shared += 1 - if not waiter.future.done(): - waiter.future.set_result(None) + waiter.future.set_result(None) # Continue granting a cohort of shared waiters up until # the first exclusive waiter. @@ -5347,13 +5394,43 @@ async def _transaction_coordinator_abort( participants: Participants, coordinator_state_ref: Optional[StateRef], ): - """Aborts a transaction for which we are the coordinator: (1) cleans - up the "preparing" record that may be in the database; (2) - best-effort sends `Abort` RPCs to all participants; and (3) - resolves the `_coordinator_participants` future to `None` so - any participant "watch control loops" observe the abort, then - deletes the future entry. + """Aborts a transaction for which we are the coordinator: (1) + resolves the `_coordinator_participants` future to `None` and + drops its entry so any participant "watch control loops" + observe the abort; (2) cleans up the "preparing" record that + may be in the database; and (3) best-effort sends `Abort` RPCs + to all participants. """ + # To indicate that the transaction has aborted we resolve the + # future to `None`, so a participant `Watch` that already passed + # the "is it still in the map" check and is awaiting the future + # observes the abort rather than finding itself still listed to + # commit. Both statements are synchronous and come before + # anything that can be interrupted, so from here on every + # watcher has an answer no matter where a cancellation lands. + # + # Answering before the database cleanup below lands is safe, + # because an abort is not something a coordinator records: its + # durable outcomes are "preparing" and "absent", and neither + # says a transaction committed. A coordinator only gets here + # while the transaction is not durably prepared -- either its + # `_transaction_coordinator_complete()` did not return, or it + # is re-preparing, which asserts as much -- so there is no + # committed outcome for an early answer to contradict. Should + # this process die before the cleanup lands, recovery finds + # "preparing" and re-prepares, the participants that have + # already aborted report that they never prepared, and the + # transaction aborts again. + # + # Not expecting the future to ever be cancelled, see: + # https://github.com/reboot-dev/mono/issues/3241 + assert not self._coordinator_participants[transaction_id].cancelled() + self._coordinator_participants[transaction_id].set_result(None) + + # Remove transaction so that participants "watch control loop" + # will determine that the transaction has aborted! + del self._coordinator_participants[transaction_id] + # Clean up the "preparing" record that may be in the database # when the coordinator stored the transaction participants # (and `preparing=True`). The database write may or may not @@ -5418,21 +5495,6 @@ async def abort(state_type: StateTypeName, state_ref: StateRef): for (state_type, state_ref) in participants.should_prepare() ) - # To indicate that the transaction has aborted we resolve the - # future to `None`, so a participant `Watch` that already passed - # the "is it still in the map" check and is awaiting the future - # observes the abort rather than finding itself still listed to - # commit. - # - # Not expecting the future to ever be cancelled, see: - # https://github.com/reboot-dev/mono/issues/3241 - assert not self._coordinator_participants[transaction_id].cancelled() - self._coordinator_participants[transaction_id].set_result(None) - - # Remove transaction so that participants "watch control loop" - # will determine that the transaction has aborted! - del self._coordinator_participants[transaction_id] - async def _transaction_participant_watch( self, application_id: ApplicationId, diff --git a/tests/reboot/agents/pydantic_ai/BUILD.bazel b/tests/reboot/agents/pydantic_ai/BUILD.bazel index bfa73596..0f1dba5c 100644 --- a/tests/reboot/agents/pydantic_ai/BUILD.bazel +++ b/tests/reboot/agents/pydantic_ai/BUILD.bazel @@ -6,6 +6,10 @@ py_test( timeout = "long", srcs = [":agent_tests.py"], main = "agent_tests.py", + # This exercises the `pydantic-ai` integration against a mocked + # model (`FunctionModel`) on the in-process `Reboot()` test + # harness; neither has platform-dependent behavior. + tags = ["platform-independent"], deps = [ requirement("mcp"), requirement("pydantic-ai-slim"), diff --git a/tests/reboot/cli/init/BUILD.bazel b/tests/reboot/cli/init/BUILD.bazel index e6882062..76ad90d6 100644 --- a/tests/reboot/cli/init/BUILD.bazel +++ b/tests/reboot/cli/init/BUILD.bazel @@ -96,7 +96,15 @@ sh_test( # Additionally, in all our example tests, we initiate LocalEnvoy on the # default secure port 9991. We require 'exclusive' to not run multiple # tests, that use the same port, concurrently. + # + # Installing the locally built Reboot npm packages, `rbt generate` + # and `npx rbt dev run` are exercised on MacOS by + # `//tests/reboot/examples/chat-room-nodejs:test`, and the `rbt + # init` flow by `init_sh_test_python310`; both keep running + # there, and what is left is the content of the Node.js + # templates. tags = [ "exclusive", + "platform-independent", ], ) diff --git a/tests/reboot/examples/agent-wiki/BUILD.bazel b/tests/reboot/examples/agent-wiki/BUILD.bazel index 3baa5b4e..0d5cee27 100644 --- a/tests/reboot/examples/agent-wiki/BUILD.bazel +++ b/tests/reboot/examples/agent-wiki/BUILD.bazel @@ -30,11 +30,6 @@ osx_env.update({ sh_test_in_working_directory( name = "test", - # MacOS runners need the full Bazel "large" budget because - # `uv sync` cold-fetches the locked dependency tree (and - # possibly a managed Python interpreter) into the test - # sandbox, which exceeds the default "medium" 300s timeout. - size = "large", data = test_packages + [ ":test_packages", # The actual test script will be given access to all of @@ -51,7 +46,16 @@ sh_test_in_working_directory( # In all our example tests, we initiate LocalEnvoy on the # default secure port 9991. We require 'exclusive' to not run # multiple tests, that use the same port, concurrently. - tags = ["exclusive"], + # + # The `uv sync`, `rbt generate`, `pytest` and `rbt dev run + # --terminate-after-health-check` steps are the same ones that + # `//tests/reboot/examples/chat-room:test` runs, and that test + # keeps running on MacOS; only the application code in between + # differs, so this adds no MacOS-specific coverage. + tags = [ + "exclusive", + "platform-independent", + ], working_directory = "reboot/examples/agent-wiki/", ) @@ -79,5 +83,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/agent-wiki/", ) diff --git a/tests/reboot/examples/ai-chat-counter-dashboard/BUILD.bazel b/tests/reboot/examples/ai-chat-counter-dashboard/BUILD.bazel index 7973464e..063915fe 100644 --- a/tests/reboot/examples/ai-chat-counter-dashboard/BUILD.bazel +++ b/tests/reboot/examples/ai-chat-counter-dashboard/BUILD.bazel @@ -37,5 +37,12 @@ sh_test_in_working_directory( # is generated from are copied into the test sandbox; the script # `cd`s into `frontend/` itself. script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/ai-chat-counter-dashboard/", ) diff --git a/tests/reboot/examples/ai-chat-counter/BUILD.bazel b/tests/reboot/examples/ai-chat-counter/BUILD.bazel index 703d1ecf..865de52e 100644 --- a/tests/reboot/examples/ai-chat-counter/BUILD.bazel +++ b/tests/reboot/examples/ai-chat-counter/BUILD.bazel @@ -37,5 +37,12 @@ sh_test_in_working_directory( # is generated from are copied into the test sandbox; the script # `cd`s into `frontend/` itself. script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/ai-chat-counter/", ) diff --git a/tests/reboot/examples/bank-pydantic/BUILD.bazel b/tests/reboot/examples/bank-pydantic/BUILD.bazel index 37b9f436..0020587d 100644 --- a/tests/reboot/examples/bank-pydantic/BUILD.bazel +++ b/tests/reboot/examples/bank-pydantic/BUILD.bazel @@ -97,7 +97,16 @@ sh_test_in_working_directory( # from are copied into the test sandbox; the script `cd`s into # `frontend/mobile/` itself. script = "frontend/mobile/.tests/react_native_test.sh", - tags = ["exclusive"], + # `npx tsc --noEmit` produces the same result on MacOS as on + # Linux; generating the mobile client is exercised on MacOS by + # `//tests/reboot/cli/generate:mobile_out_specified_tests_py`, + # and installing the locally built Reboot npm packages by + # `//tests/reboot/examples/chat-room-nodejs:test`; both keep + # running there. + tags = [ + "exclusive", + "platform-independent", + ], working_directory = "reboot/examples/bank-pydantic/", ) @@ -135,5 +144,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/bank-pydantic/", ) diff --git a/tests/reboot/examples/bank-zod/BUILD.bazel b/tests/reboot/examples/bank-zod/BUILD.bazel index f6c839f9..1e70c9fd 100644 --- a/tests/reboot/examples/bank-zod/BUILD.bazel +++ b/tests/reboot/examples/bank-zod/BUILD.bazel @@ -70,5 +70,12 @@ sh_test_in_working_directory( ], env = general_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/bank-zod/", ) diff --git a/tests/reboot/examples/bank/BUILD.bazel b/tests/reboot/examples/bank/BUILD.bazel index d8b2f682..c7ae8562 100644 --- a/tests/reboot/examples/bank/BUILD.bazel +++ b/tests/reboot/examples/bank/BUILD.bazel @@ -72,5 +72,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/bank/", ) diff --git a/tests/reboot/examples/boutique/BUILD.bazel b/tests/reboot/examples/boutique/BUILD.bazel index cd54b944..1b979ff5 100644 --- a/tests/reboot/examples/boutique/BUILD.bazel +++ b/tests/reboot/examples/boutique/BUILD.bazel @@ -80,5 +80,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/boutique/", ) diff --git a/tests/reboot/examples/chat-room/BUILD.bazel b/tests/reboot/examples/chat-room/BUILD.bazel index de5980d2..fe42dddf 100644 --- a/tests/reboot/examples/chat-room/BUILD.bazel +++ b/tests/reboot/examples/chat-room/BUILD.bazel @@ -166,7 +166,16 @@ sh_test_in_working_directory( # from are copied into the test sandbox; the script `cd`s into # `frontend/mobile/` itself. script = "frontend/mobile/.tests/react_native_test.sh", - tags = ["exclusive"], + # `npx tsc --noEmit` produces the same result on MacOS as on + # Linux; generating the mobile client is exercised on MacOS by + # `//tests/reboot/cli/generate:mobile_out_specified_tests_py`, + # and installing the locally built Reboot npm packages by + # `//tests/reboot/examples/chat-room-nodejs:test`; both keep + # running there. + tags = [ + "exclusive", + "platform-independent", + ], working_directory = "reboot/examples/chat-room/", ) @@ -205,5 +214,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/chat-room/", ) diff --git a/tests/reboot/examples/chick-potle/BUILD.bazel b/tests/reboot/examples/chick-potle/BUILD.bazel index 73e171d1..1b09337f 100644 --- a/tests/reboot/examples/chick-potle/BUILD.bazel +++ b/tests/reboot/examples/chick-potle/BUILD.bazel @@ -74,5 +74,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/chick-potle/", ) diff --git a/tests/reboot/examples/kcdc-2025/BUILD.bazel b/tests/reboot/examples/kcdc-2025/BUILD.bazel index c957850f..0e77f382 100644 --- a/tests/reboot/examples/kcdc-2025/BUILD.bazel +++ b/tests/reboot/examples/kcdc-2025/BUILD.bazel @@ -29,5 +29,12 @@ sh_test_in_working_directory( ], env = frontend_env, script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/kcdc-2025/", ) diff --git a/tests/reboot/examples/monorepo/BUILD.bazel b/tests/reboot/examples/monorepo/BUILD.bazel index 1d2a08f6..56567d14 100644 --- a/tests/reboot/examples/monorepo/BUILD.bazel +++ b/tests/reboot/examples/monorepo/BUILD.bazel @@ -56,6 +56,15 @@ sh_test_in_working_directory( # In all our example tests, we initiate LocalEnvoy on the default # secure port 9991. We require 'exclusive' to not run multiple # tests, that use the same port, concurrently. - tags = ["exclusive"], + # + # `uv sync`, `rbt generate` and the in-process `pytest` harness + # are exercised on MacOS by `//tests/reboot/examples/bank:test`, + # which keeps running there; what is left here is `rbt generate` + # resolving paths from the sub-directories of a multi-application + # workspace, which is pure Python. + tags = [ + "exclusive", + "platform-independent", + ], working_directory = "reboot/examples/monorepo/", ) diff --git a/tests/reboot/examples/prosemirror-zod/BUILD.bazel b/tests/reboot/examples/prosemirror-zod/BUILD.bazel index c012c527..779081dc 100644 --- a/tests/reboot/examples/prosemirror-zod/BUILD.bazel +++ b/tests/reboot/examples/prosemirror-zod/BUILD.bazel @@ -47,8 +47,15 @@ sh_test_in_working_directory( # Additionally, in all our example tests, we initiate LocalEnvoy on the # default secure port 9991. We require 'exclusive' to not run multiple # tests, that use the same port, concurrently. + # + # `.tests/test.sh` here is identical to the one that + # `//tests/reboot/examples/prosemirror:test` runs on MacOS; + # only the schema style (zod instead of `.proto`) and the + # generated client that follows from it differ, which is + # codegen rather than platform behavior. tags = [ "exclusive", + "platform-independent", ], working_directory = "reboot/examples/prosemirror-zod", ) diff --git a/tests/reboot/examples/reboot-swag-store/BUILD.bazel b/tests/reboot/examples/reboot-swag-store/BUILD.bazel index 9d775379..31337380 100644 --- a/tests/reboot/examples/reboot-swag-store/BUILD.bazel +++ b/tests/reboot/examples/reboot-swag-store/BUILD.bazel @@ -85,5 +85,12 @@ sh_test_in_working_directory( # is generated from are copied into the test sandbox; the script # `cd`s into `frontend/` itself. script = "frontend/.tests/type_check.sh", + # `tsc` and the frontend bundler produce the same result on MacOS + # as on Linux, and the `rbt generate` and `npm install` steps on + # the way there are exercised on MacOS by + # `//tests/reboot/examples/chat-room:test` and + # `//tests/reboot/examples/chat-room-nodejs:test`, which keep + # running there. + tags = ["platform-independent"], working_directory = "reboot/examples/reboot-swag-store/", ) diff --git a/tests/reboot/react/test_cache/BUILD.bazel b/tests/reboot/react/test_cache/BUILD.bazel index 6ce48ad1..0dffb505 100644 --- a/tests/reboot/react/test_cache/BUILD.bazel +++ b/tests/reboot/react/test_cache/BUILD.bazel @@ -137,6 +137,14 @@ vitest_test( # from. env = {"REBOOT_WHL_FILE": "$(rootpath //reboot:reboot.dev)"}, node_modules = "//:node_modules", + # The React client logic under test runs in `vitest` under + # `jsdom`, with no browser involved; the `chromium` `web_test`s in + # `//tests/reboot/react/...` carry `macos_not_supported`. The + # Reboot backend and LocalEnvoy that `rbt.up()` starts are + # exercised on MacOS by + # `//tests/reboot/nodejs/reboot_web_test:test`, which keeps + # running there. + tags = ["platform-independent"], visibility = ["//visibility:public"], ) @@ -155,6 +163,14 @@ vitest_test( # from. env = {"REBOOT_WHL_FILE": "$(rootpath //reboot:reboot.dev)"}, node_modules = "//:node_modules", + # The React client logic under test runs in `vitest` under + # `jsdom`, with no browser involved; the `chromium` `web_test`s in + # `//tests/reboot/react/...` carry `macos_not_supported`. The + # Reboot backend and LocalEnvoy that `rbt.up()` starts are + # exercised on MacOS by + # `//tests/reboot/nodejs/reboot_web_test:test`, which keeps + # running there. + tags = ["platform-independent"], visibility = ["//visibility:public"], ) @@ -173,6 +189,14 @@ vitest_test( # from. env = {"REBOOT_WHL_FILE": "$(rootpath //reboot:reboot.dev)"}, node_modules = "//:node_modules", + # The React client logic under test runs in `vitest` under + # `jsdom`, with no browser involved; the `chromium` `web_test`s in + # `//tests/reboot/react/...` carry `macos_not_supported`. The + # Reboot backend and LocalEnvoy that `rbt.up()` starts are + # exercised on MacOS by + # `//tests/reboot/nodejs/reboot_web_test:test`, which keeps + # running there. + tags = ["platform-independent"], visibility = ["//visibility:public"], ) diff --git a/tests/reboot/react/test_mutations/BUILD.bazel b/tests/reboot/react/test_mutations/BUILD.bazel index 95d8fabf..765adb67 100644 --- a/tests/reboot/react/test_mutations/BUILD.bazel +++ b/tests/reboot/react/test_mutations/BUILD.bazel @@ -51,6 +51,11 @@ js_reboot_react_library( # might want to revisit this). build_test( name = "test_build_no_mutations", + # Compiling generated TypeScript behaves the same on MacOS as on + # Linux, and generating a React client is exercised on MacOS by + # `//tests/reboot/cli/generate:react_out_specified_tests_py`, + # which keeps running there. + tags = ["platform-independent"], targets = [":test_js_reboot_react"], ) diff --git a/tests/reboot/react/test_ordered_map/BUILD.bazel b/tests/reboot/react/test_ordered_map/BUILD.bazel index c0d51f1c..c2ced64a 100644 --- a/tests/reboot/react/test_ordered_map/BUILD.bazel +++ b/tests/reboot/react/test_ordered_map/BUILD.bazel @@ -47,5 +47,13 @@ vitest_test( # from. env = {"REBOOT_WHL_FILE": "$(rootpath //reboot:reboot.dev)"}, node_modules = "//:node_modules", + # The React client logic under test runs in `vitest` under + # `jsdom`, with no browser involved; the `chromium` `web_test`s in + # `//tests/reboot/react/...` carry `macos_not_supported`. The + # Reboot backend and LocalEnvoy that `rbt.up()` starts are + # exercised on MacOS by + # `//tests/reboot/nodejs/reboot_web_test:test`, which keeps + # running there. + tags = ["platform-independent"], visibility = ["//visibility:public"], ) diff --git a/tests/reboot/react/test_state_id_change/BUILD.bazel b/tests/reboot/react/test_state_id_change/BUILD.bazel index 26c5d5d0..4dc97a66 100644 --- a/tests/reboot/react/test_state_id_change/BUILD.bazel +++ b/tests/reboot/react/test_state_id_change/BUILD.bazel @@ -49,5 +49,13 @@ vitest_test( # from. env = {"REBOOT_WHL_FILE": "$(rootpath //reboot:reboot.dev)"}, node_modules = "//:node_modules", + # The React client logic under test runs in `vitest` under + # `jsdom`, with no browser involved; the `chromium` `web_test`s in + # `//tests/reboot/react/...` carry `macos_not_supported`. The + # Reboot backend and LocalEnvoy that `rbt.up()` starts are + # exercised on MacOS by + # `//tests/reboot/nodejs/reboot_web_test:test`, which keeps + # running there. + tags = ["platform-independent"], visibility = ["//visibility:public"], ) diff --git a/tests/reboot/state_manager_tests.py b/tests/reboot/state_manager_tests.py index 04b8166b..8b120a93 100644 --- a/tests/reboot/state_manager_tests.py +++ b/tests/reboot/state_manager_tests.py @@ -1131,6 +1131,236 @@ async def test_exclusive_deadline_raises_unavailable(self) -> None: self.assertFalse(lock.is_exclusive_locked()) lock.release_shared() + async def test_cancelled_exclusive_waiter_leaves_lock_free(self) -> None: + """A queued exclusive waiter whose task is cancelled before it + is granted must not be handed the exclusive hold. Its acquire + can only raise from here on, so nothing will ever release that + hold: the lock would stay held by nobody for the life of the + process, and every later acquire on this state would fail or + block forever.""" + lock = Lock() + await lock.acquire_shared(deadline=None) + + async def exclusive() -> None: + await lock.acquire_exclusive(deadline=None) + lock.release_exclusive() + + exclusive_task = asyncio.create_task(exclusive()) + # One `sleep(0)` is enough to leave the task queued on the + # lock. `create_task()` puts the task's first run on the event + # loop's ready queue, and `sleep(0)` puts this coroutine's own + # continuation on that same queue behind it; the queue is + # first in, first out, so the task runs before we resume. That + # first run carries the task to its first suspension, which is + # `Lock._wait()` awaiting the waiter's future -- everything + # ahead of that, putting the waiter on the lock's queue + # included, is synchronous. + await asyncio.sleep(0) + + exclusive_task.cancel() + lock.release_shared() + with self.assertRaises(asyncio.CancelledError): + await exclusive_task + + self.assertFalse(lock.is_locked()) + # The lock is genuinely free, i.e., a fresh acquire is granted + # right away rather than queueing behind a holder that is not + # there. + self.assertTrue(lock.try_acquire_exclusive()) + lock.release_exclusive() + + async def test_cancelled_shared_waiter_leaves_lock_free(self) -> None: + """The shared counterpart: a cancelled shared waiter must not + be counted as a holder. It would be counted for the life of + the process, blocking every later exclusive acquire and every + upgrade on this state forever.""" + lock = Lock() + await lock.acquire_exclusive(deadline=None) + + async def shared() -> None: + await lock.acquire_shared(deadline=None) + lock.release_shared() + + shared_task = asyncio.create_task(shared()) + # Runs the task up to queueing on the lock. + await asyncio.sleep(0) + + shared_task.cancel() + lock.release_exclusive() + with self.assertRaises(asyncio.CancelledError): + await shared_task + + self.assertFalse(lock.is_locked()) + self.assertTrue(lock.try_acquire_exclusive()) + lock.release_exclusive() + + async def test_cancelled_upgrader_leaves_lock_shared(self) -> None: + """A cancelled upgrader must not be handed the exclusive hold + either, which would wedge the lock exclusively forever. It + keeps the shared hold it was upgrading from.""" + lock = Lock() + # Two shared holders, so the upgrade has to queue. + await lock.acquire_shared(deadline=None) + await lock.acquire_shared(deadline=None) + + async def upgrader() -> None: + await lock.upgrade(deadline=None) + lock.release_exclusive() + + upgrader_task = asyncio.create_task(upgrader()) + # Runs the task up to queueing the upgrade on the lock. + await asyncio.sleep(0) + + upgrader_task.cancel() + # Releasing leaves a single shared holder, which is exactly the + # condition under which a pending upgrade is granted. + lock.release_shared() + with self.assertRaises(asyncio.CancelledError): + await upgrader_task + + self.assertTrue(lock.is_shared_locked()) + self.assertFalse(lock.is_exclusive_locked()) + lock.release_shared() + self.assertFalse(lock.is_locked()) + + async def test_cancelled_granted_upgrader_keeps_its_shared_hold( + self, + ) -> None: + """An upgrade that is granted and whose caller is then + interrupted before it can use the exclusive hold leaves that + caller believing it still holds shared, so that is what the + lock must be left holding. Releasing the exclusive hold + outright instead would drop a shared hold its caller still + owes, and that caller's later release would assert.""" + lock = Lock() + await lock.acquire_shared(deadline=None) + await lock.acquire_shared(deadline=None) + + async def upgrader() -> None: + await lock.upgrade(deadline=None) + lock.release_exclusive() + + upgrader_task = asyncio.create_task(upgrader()) + # Runs the task up to queueing the upgrade on the lock. + await asyncio.sleep(0) + + # Grant the upgrade, then interrupt its caller before it gets + # a chance to run again. + lock.release_shared() + self.assertTrue(lock.is_exclusive_locked()) + upgrader_task.cancel() + with self.assertRaises(asyncio.CancelledError): + await upgrader_task + + self.assertTrue(lock.is_shared_locked()) + self.assertFalse(lock.is_exclusive_locked()) + lock.release_shared() + self.assertFalse(lock.is_locked()) + + async def test_cancelled_upgrader_leaves_a_later_upgrade_alone( + self, + ) -> None: + """A cancelled upgrader is dropped from the upgrade slot as + soon as a release notices it, which frees the slot for another + holder to register an upgrade of its own. When the cancelled + one is resumed to clean up after itself it must leave that + later upgrade in place: emptying the slot would leave the + later upgrade registered nowhere, so nothing could ever grant + it and its caller would wait out its whole acquire deadline + for a hold that was free the entire time.""" + lock = Lock() + # A shared hold each for this test, the cancelled upgrader and + # the later upgrader, so that neither upgrade is ever the sole + # holder and both have to queue. + await lock.acquire_shared(deadline=None) + + async def cancelled_upgrader() -> None: + await lock.acquire_shared(deadline=None) + try: + await lock.upgrade(deadline=None) + except BaseException: + # An upgrade that fails leaves its caller holding the + # shared hold it was upgrading from. + lock.release_shared() + raise + lock.release_exclusive() + + cancelled_task = asyncio.create_task(cancelled_upgrader()) + # Runs the task up to registering its upgrade. + await asyncio.sleep(0) + + async def later_upgrader() -> None: + await lock.acquire_shared(deadline=None) + # A deadline rather than `None`, so that an upgrade left + # registered nowhere fails this test rather than hanging + # the suite. The upgrade is granted while the cancelled + # upgrader unwinds, well within it. + await lock.upgrade(deadline=timedelta(seconds=5)) + lock.release_exclusive() + + # Everything from here to the `sleep(0)` below is synchronous, + # which is what fixes the order the event loop runs these in: + # the later upgrader's first run is queued first, the + # cancelled upgrader's wake-up second, and this coroutine's + # own continuation last, on a first in, first out queue. + later_task = asyncio.create_task(later_upgrader()) + cancelled_task.cancel() + # Releasing notices the cancelled upgrade and empties the + # slot, leaving it free for the later upgrader to claim. + lock.release_shared() + + # Runs the later upgrader, which takes a shared hold and + # registers its upgrade in the free slot, and then the + # cancelled upgrader, which cleans up and hands back its own + # shared hold, leaving the later upgrade the only reason this + # lock is still held. + await asyncio.sleep(0) + + with self.assertRaises(asyncio.CancelledError): + await cancelled_task + + # The later upgrade was granted as the cancelled upgrader + # unwound, so this returns rather than timing out. + await later_task + self.assertFalse(lock.is_locked()) + + async def test_waiter_queued_behind_a_cancelled_one_is_granted( + self, + ) -> None: + """Dropping a cancelled waiter lets whoever queued behind it + through, rather than stalling the rest of the queue.""" + lock = Lock() + await lock.acquire_exclusive(deadline=None) + + async def cancelled_shared() -> None: + await lock.acquire_shared(deadline=None) + lock.release_shared() + + cancelled_task = asyncio.create_task(cancelled_shared()) + # Runs the task up to queueing on the lock. + await asyncio.sleep(0) + + shared_acquired = asyncio.Event() + + async def shared() -> None: + await lock.acquire_shared(deadline=None) + shared_acquired.set() + + shared_task = asyncio.create_task(shared()) + # Runs the task up to queueing behind the first one. + await asyncio.sleep(0) + + cancelled_task.cancel() + lock.release_exclusive() + with self.assertRaises(asyncio.CancelledError): + await cancelled_task + + await shared_acquired.wait() + await shared_task + self.assertTrue(lock.is_shared_locked()) + lock.release_shared() + self.assertFalse(lock.is_locked()) + class EffectsRequiresExclusiveTest(unittest.TestCase): """Unit tests for `Effects.requires_exclusive()`, which decides diff --git a/tests/reboot/std/collections/ordered_map/v1/BUILD.bazel b/tests/reboot/std/collections/ordered_map/v1/BUILD.bazel index 8047cb2d..eb312aa7 100644 --- a/tests/reboot/std/collections/ordered_map/v1/BUILD.bazel +++ b/tests/reboot/std/collections/ordered_map/v1/BUILD.bazel @@ -7,6 +7,12 @@ py_test( size = "large", srcs = [":ordered_map_tests.py"], main = "ordered_map_tests.py", + # This exercises the `OrderedMap` B-tree logic on top of the + # in-process `Reboot()` test harness, neither of which behaves + # differently on MacOS; the storage engine underneath is + # exercised on MacOS by `//tests/reboot/server:database_tests` + # and `//tests/reboot:state_manager_tests_py`. + tags = ["platform-independent"], deps = [ "//rbt/std/item/v1:item_py_reboot", "//reboot:protobuf_py", diff --git a/tests/reboot/zod/concurrent_transactions_same_state/BUILD.bazel b/tests/reboot/zod/concurrent_transactions_same_state/BUILD.bazel index 90e4f205..e226721b 100644 --- a/tests/reboot/zod/concurrent_transactions_same_state/BUILD.bazel +++ b/tests/reboot/zod/concurrent_transactions_same_state/BUILD.bazel @@ -55,6 +55,12 @@ ts_project( js_reboot_test( name = "test", + # A Node test builds a Python virtualenv and `pip install`s the + # Reboot wheel into it before any of the test body runs. On the + # MacOS arm64 CI runner that alone has taken the whole 300s a + # `medium` test gets, killing the test before it started, so this + # one gets the 900s of a `large` test to work in. + size = "large", data = [ ":test_ts", ],