Commit Graph
30 Commits
Author SHA1 Message Date
Dmitry VerkhoturovandGitHub 49bf83b09c Address the review follow-ups from #2188, #2189 and #2190 (#2193)
* Read the collapsed-threads key through getJsonItem

`getFromLocalStorage` parsed the stored string directly, so anything
malformed under `__remarkCollapsed` threw out of `restoreCollapsedThreads`.
That call sits in `remark.tsx` ahead of the `render`, so the throw took the
whole widget with it: the reader was left on the preloader, over a view
preference.

`getJsonItem` in `common/local-storage.ts` already wraps a parse of a
localStorage key and returns null on failure, and null is a shape the check
below already reads as empty. The rest of that function is total against
whatever the browser holds, and the bare parse was the one way in.

* Stop retrying a failed e2e test in CI

The suite went in with one gotestsum rerun. It has no failures on record to
justify that: 31 CI runs since it landed, all green, and no rerun report has
ever been produced. A retry is what turns an intermittent regression into a
green build, and while the suite is this young its own failures are the
evidence worth keeping.

`E2E_RUN_ID` stays. It stamps the threads a run works on with the CI run id, so
a thread url in a trace or a log names the run it came from. It carries no data
across: the stack is disposable, and a local run under the same id gets those
urls on an empty database.

* Stop two chooseUnusedPort comments claiming collisions cannot happen

All four copies listen on :0, read the assigned port, close the listener
and bind later, so nothing holds the number across that gap and another
binary can take it. The copies in app/cmd and app/rest/api call a collision
very unlikely, which is accurate; the ones in app and the example module
said binaries never land on the same number, which is not, and a comment
ruling out a port collision is what would send the next person chasing one
somewhere else. All four now read the same.

Closing the window rather than describing it means the server binding :0
itself and reporting the address it got, which is a larger change.
2026-08-22 02:23:56 -05:00
Dmitry VerkhoturovandGitHub 0b651dddd4 Make backend tests wait on conditions instead of durations (#2190)
* Make backend tests wait on conditions instead of durations

The backend workflow has a long tail of runs that fail once and pass on
a rerun. Every one of them comes down to a test assuming an operation
finishes within some duration rather than waiting for the state it
needs. Three were reproducible and each was reproduced against the old
code before being changed: TestServerAuthHooks minted a token that lived
one second and never tested expiry, so a slow runner turned the first
POST into a 401; TestServerApp_AnonMode saw "connection refused" because
waitForHTTPServerStart returned silently after three seconds and left a
later assertion to fail with something unrelated; TestFsStore_Cleanup
slept 200ms against a 300ms ttl that Cleanup widens to 400ms with its
commit grace, so roughly 100ms of stall collected an image meant to
survive.

Fixed sleeps before asserting on asynchronous work are replaced with
polls on the condition itself, using require.Eventually and
require.EventuallyWithT, and require.Never where the assertion is that
something did not happen. Polling closures assert on the CollectT they
are handed rather than on t, since testify runs them on another
goroutine, and polls that issue HTTP requests stay under the rate limit
on the routes they poll through.

Where a test needs time to have passed, the clock input is pinned
instead: staging ages are stamped with os.Chtimes on both sides of the
cleanup boundary right before each call, which also makes the 100ms
commit grace an exact case rather than something no assertion reaches,
and the RSS tests set store.Comment.Timestamp explicitly rather than
racing the wall clock into the first 100ms of a second so pubDate
matches.

chooseUnusedPort takes a port from the kernel's ephemeral range. Picking
at random out of a fixed 10000-port window let two package binaries,
which go test ./... runs concurrently, land on the same number between
the probe closing and the server binding. The start helpers fail naming
the port they waited on, and the SSL tests wait on the redirect port as
well as the TLS one.

Arbitrary budgets that nothing tests are gone: ten HTTP clients with a
one-second timeout against bolt-backed import and export, the "should
take about 100msec" assertions, and a one-second bound on noticing an
already cancelled context. Shutdown stays bounded at ten seconds so a
hang is still caught.

Two assertions get stronger. TestServerAuthHooks accepted 403 or 401
from a blocked user, an alternative that existed only because the short
token could expire mid-test; it is deterministically 403 now.
TestAdmin_BlockedList asserted two users blocked while one carried the
same 150ms ttl the next step waits to lapse, so the halves raced each
other.

goleak stops reporting the regexp2 clock goroutine, which chroma pulls
in for syntax highlighting and which lives for up to a second after the
last match with a timeout; it ends on its own but a binary finishing
inside that window was reported as leaking, and this suite now finishes
sooner. The ignore for net/http.(*Server).Shutdown goes the other way:
it no longer matches anything, with both packages run fifteen times each
under CPU oversubscription to confirm.

Two gaps the change would otherwise have opened are covered directly
rather than left to the side effects that used to cover them. The
one-second token was the only thing exercising the authenticator's
ClaimsUpd hook on refresh, so TestServerApp_ClaimsUpd now calls the hook
itself and checks admin, blocked, email and restricted-name
impersonation, including the two pass-through cases. Lifting the
open-route limit removed the last incidental exercise of the rate
limiter, so TestRateLimiter drives a burst past the allowance and checks
the refusals and that the limit is per client. Both run without a wall
clock, and both were confirmed to fail when the behaviour they cover is
removed.

Production code is untouched. The two sleeps outside test code, the 429
backoff in cmd/cleanup.go and the submit poll in store/image/image.go,
are left alone: no CI failure implicates them.

Test sleeps drop from 67 to 21, all of them either inside a
testing/synctest bubble or a poll interval. The suite runs in about 22
seconds instead of 46, mostly because
TestPublic_FindCommentsCtrl_ConsistentCount no longer paces a hundred
subtests with an 80ms sleep each to stay under the open route limit. The
300s per-package budget now matches across both workflows, the race_test
target and the documented command, and CLAUDE.md records the convention.

with '#' will be ignored, and an empty message aborts the commit. # #
Date: Sat Aug 22 01:12:31 2026 +0100 # # interactive rebase in progress;
onto 7c312da1 # Last command done (1 command done): # reword deb6cbf1 #
Make backend tests wait on conditions instead of durations # Next
command to do (1 remaining command): # reword 262e6dc2 # Apply go fix
under Go 1.27 # You are currently editing a commit while rebasing branch
'fix/backend-test-flakiness' on '7c312da1'. # # Changes to be committed:
.github/workflows/release.yml # modified: CLAUDE.md # modified: Makefile
modified: backend/_example/memory_store/server/rpc_test.go # modified:
backend/app/cmd/import_test.go # modified:
backend/app/cmd/server_test.go # modified: backend/app/main_test.go #
modified: backend/app/rest/api/admin_test.go # modified:
backend/app/rest/api/middleware_test.go # modified:
backend/app/rest/api/migrator_test.go # modified:
backend/app/rest/api/rest_private_test.go # modified:
backend/app/rest/api/rest_public_test.go # modified:
backend/app/rest/api/rest_test.go # modified:
backend/app/rest/api/rss_test.go # modified:
backend/app/rest/proxy/image_test.go # modified:
backend/app/store/image/fs_store_test.go # modified:
backend/app/store/service/service_test.go # modified:
docs/backlog/api-tests-deadlock-on-macos.md #

* Apply go fix under Go 1.27

Go 1.27 extends go fix with the modernizers, so `go fix ./...` now
rewrites patterns the language has since replaced. Running it across all
three modules produces this: legacy sync/atomic calls on plain integers
become the atomic types (notify.Service.closed, image.Service.term and
submitCount, and several test counters), reverse index loops become
slices.Backward, a Split-then-index becomes strings.Cut, counted loops
become range over an int, and interface{} becomes any in the e2e suite.

The example module needed no changes. The e2e module is behind a build
tag, so it only matches with `go fix -tags e2e ./...`.

One knock-on: prealloc can see the bound of a loop once it is written as
range over an int, so the slice it feeds is now preallocated.

with '#' will be ignored, and an empty message aborts the commit. # #
Date: Sat Aug 22 01:32:09 2026 +0100 # # interactive rebase in progress;
onto 7c312da1 # Last commands done (2 commands done): # reword deb6cbf1
262e6dc2 # Apply go fix under Go 1.27 # No commands remaining. # You are
currently editing a commit while rebasing branch
'fix/backend-test-flakiness' on '7c312da1'. # # Changes to be committed:
backend/app/migrator/native.go # modified: backend/app/notify/notify.go
backend/app/rest/api/rest_private_test.go # modified:
backend/app/store/comment.go # modified:
backend/app/store/image/image.go # modified:
backend/app/store/service/service_test.go # modified:
backend/app/store/service/title_test.go # modified: e2e/e2e_test.go #
modified: e2e/widgets_test.go #
2026-08-21 22:17:44 -05:00
Dmitry VerkhoturovandGitHub ba7c3aed94 refactor: modernise Go code with go fix and manual improvements (#2027)
Apply go fix ./... analysers (Go 1.26) across backend and examples:
- interface{} → any (type alias, no behaviour change)
- for i := 0; i < N; i++ → for range N / for i := range N
- slices.Contains / slices.ContainsFunc replacing manual loops
- strings.SplitSeq replacing strings.Split in range (avoids allocation)
- strings.CutPrefix replacing HasPrefix+TrimPrefix
- min() replacing manual if/else
- fmt.Appendf replacing []byte(fmt.Sprintf(...))
- strings.Builder replacing string += concatenation
- wg.Go(func(){}) replacing wg.Add(1)/go/wg.Done() pattern
- removed redundant ii := i loop variable copies (unnecessary since Go 1.22)

omitempty on struct-typed JSON fields: go fix removed omitempty from
struct-typed fields (time.Time, PostInfo, UserDetailEntry) because
encoding/json's omitempty never applied to struct types — it was always
a no-op. Kept as bare tags (no omitzero replacement) to preserve the
existing serialisation behaviour.
2026-03-25 16:42:37 -05:00
Dmitry VerkhoturovandUmputun 81c30e01f8 cleanup images from deleted comments
Previously, images were deleted only from comments deleted
before EditDuration expiration. After this change, any deletion
of the comment deletes images if they are not used elsewhere
in comments under the same page.
2024-01-20 13:29:06 -06:00
Dmitry VerkhoturovandUmputun d7e9be99f9 make Close() calls idempotent
Previously, few of them resulted in panics when called more than once.
2023-01-03 01:41:26 -06:00
Dmitry VerkhoturovandUmputun 2ecc80e18c fix RPC engine work with ListFlags method
Fixes the following conversion problem for BlockedUser:

```
panic: interface conversion: interface {} is map[string]interface {},
not store.BlockedUser [recovered]
```

Resolves #1475.
2022-09-29 11:56:12 -05:00
Dmitry VerkhoturovandUmputun 50785e0577 bump didip/tollbooth from v6 to v7 2022-07-19 17:13:02 -05:00
Dmitry VerkhoturovandUmputun 90e537358d update golangci-lint to 1.43.0, fix found issues 2021-11-23 15:00:40 -06:00
Dmitry VerkhoturovandUmputun 0e550e83fa add method to renew image cleanup timer 2021-05-24 17:33:59 -05:00
Umputun 807160dcf6 prep admin to support multiple keys, per aud (siteID) 2021-02-15 13:14:23 -06:00
Umputun 0895de2f7e rename all packages from the legacy remark/ to remark42 2020-05-12 11:34:16 -05:00
Dmitry Verkhoturov c7f73a6c84 add image.Store.Info() function 2020-04-23 00:52:36 +02:00
Dmitry Verkhoturov 318feb8ed2 golangci-lint fixes for memory_store tests 2020-04-21 11:47:15 +02:00
Dmitry VerkhoturovandUmputun 2acb00d424 rename image.Store.SaveWithID to Save 2020-04-19 16:21:09 -05:00
Dmitry VerkhoturovandUmputun 36cee9cc66 simplify image.Store interface 2020-04-19 16:21:09 -05:00
Dmitry Verkhoturov 828669ac95 remove t.Log from test server.Run in memory_store 2020-04-14 00:20:42 +02:00
Dmitry VerkhoturovandUmputun afc94adc1b remove log package usage from tests, sort imports 2020-04-13 16:01:51 -05:00
Dmitry VerkhoturovandUmputun 0355ba4ff7 add image storage to memory_store 2020-04-04 14:06:39 -05:00
Dmitry VerkhoturovandGitHub c9a23698ba Image interface changes (#623)
@paskal 

* sort imports, add missing copyright
* regenerate engine mock
* make all image.Store interface functions public
* go mod tidy
* make image.Store.Load return []byte instead of io.ReadCloser
* separate memory_store example RPC server to multiple files by handlers groups
2020-03-22 20:43:43 -05:00
Dmitry VerkhoturovandUmputun fddb737657 Improve TestServer* tests reliability (#508)
* explicitly ignore error from test connection close

* add client with timeout to places which used default http client

* move random port creation and waiting for server in separate function for reuse

* improve TestServer tests robustness

* move all server waiting code in tests to separate functions

* change chooseRandomUnusedPort to try to listen to port before return

* fix waitForHTTPSServerStart
2019-12-29 20:46:22 -06:00
Dmitry VerkhoturovandUmputun b82be0cc2d Check if the port already in use in TestRPC_ tests server startup (#506)
* check if the port already in use in TestRPC_ tests server startup

* check for open port instead of for HTTP server listening
2019-12-29 18:24:42 -06:00
Dmitry VerkhoturovandUmputun 36b4f6774c make TestRPC tests reliably wait for HTTP server to start (#505)
* make TestRPC tests reliably wait for HTTP server to start

* adjust the wait period to be limited by 3 seconds
2019-12-29 16:23:47 -06:00
Dmitry VerkhoturovandUmputun ddd466ec41 implement user details storage (#469)
* implement (strings) user details storage

* add rpc user details implementation

* return error from getUserDetail, rewrite tests to table tests

* make UserDetails store UserDetailEntry instead of strings

* update comment about user_details

* fix confusing return

* add user details support for memory store

* add engine.UserDetailEntry to service.UserMetaData

* add ListDetails support to memory storage

* add user details support to native migrator, ListDetails func to storage

* go mod tidy for memory storage

* increase memory storage test coverage, fix tests naming

* add ListDetails tests to memory storage

* add engine.ListDetails and  service.[Set]Metas tests

* change Fprintf to Fprint (triggered by explicitly ignoring error)

* remove Delete from engine.UserDetail, implement list via same method

* adjust service.Metas to new engine.UserDetails signature

* introduce engine.UserDetail("all") consonant

* fix Meta user detail retrieval

* extend store implementations Delete method with UserDetail deletion

* make UserDetail test answer order-independent

* fix flaky test check in TestMemData_FlagListBlocked

* delete user details alongside with comments on deleteme request

* add tests to UserDetail store.Delete implementations

* clarify engine module user details consonants names

* update comments to reflect current state of code

* check for value absence instead of it's length

* revert unneeded code change

* add extensive commentary on UserDetail return type

* remove unused check condition

* clarify UserDetail tests to be truly stateless

* add clarifying comment for pre-table test
2019-11-22 02:24:26 -06:00
Dmitry VerkhoturovandUmputun 773da16649 Improve memory store tests reliability (#479)
* remove unnecessary GIT_BRANCH export

* go mod tidy

* fix tests naming, improve coverage

* make flaky TestMemData_FlagListBlocked reliable
2019-11-17 15:47:01 -06:00
UmputunandGitHub 776b9117fc CI changes (#457)
* make tests more resilient

* adjust drone for v1.x and dev-box, remove travis support

* add backend build step on branch for drone

* fix from email

* change notif email

* less demanding test wait

* add settings section to drone plugins

* adjust branch build

* convert all drone ci docker confs

* rename drone targets
2019-10-31 20:20:18 -05:00
Umputun 68235164ff add OnEvent event to admin on each update/create/vote/delete 2019-08-26 20:53:04 -05:00
Umputun e9414f7267 fix comment typo 2019-08-26 00:59:26 -05:00
Umputun 106c018ef1 mem store example updated with adm.Enabled support 2019-08-26 00:58:27 -05:00
Umputun 796c4f45eb fix rpc deps 2019-08-01 01:49:38 -05:00
Umputun e84a155ef5 switch example to go-pkgz/jrpc 2019-07-31 18:34:32 -05:00