Commit graph

8 commits

Author SHA1 Message Date
Jesse Hallam
67568d558f
Introduce model.AssertNotSameMap (#34058)
Some checks are pending
API / build (push) Waiting to run
Server CI / Compute Go Version (push) Waiting to run
Server CI / Check mocks (push) Blocked by required conditions
Server CI / Check go mod tidy (push) Blocked by required conditions
Server CI / check-style (push) Blocked by required conditions
Server CI / Check serialization methods for hot structs (push) Blocked by required conditions
Server CI / Vet API (push) Blocked by required conditions
Server CI / Check migration files (push) Blocked by required conditions
Server CI / Generate email templates (push) Blocked by required conditions
Server CI / Check store layers (push) Blocked by required conditions
Server CI / Check mmctl docs (push) Blocked by required conditions
Server CI / Postgres with binary parameters (push) Blocked by required conditions
Server CI / Postgres (push) Blocked by required conditions
Server CI / Postgres (FIPS) (push) Blocked by required conditions
Server CI / Generate Test Coverage (push) Blocked by required conditions
Server CI / Run mmctl tests (push) Blocked by required conditions
Server CI / Run mmctl tests (FIPS) (push) Blocked by required conditions
Server CI / Build mattermost server app (push) Blocked by required conditions
Web App CI / check-lint (push) Waiting to run
Web App CI / check-i18n (push) Waiting to run
Web App CI / check-types (push) Waiting to run
Web App CI / test (push) Waiting to run
Web App CI / build (push) Waiting to run
When we last bumped dependencies in https://github.com/mattermost/mattermost/pull/30005, `assert.NotSame` for maps started failing because of the change in https://github.com/stretchr/testify/issues/1661. The reality was that the previous assertion was silently skipped, and just now reporting as much.

Here's an illustrative example:
```go
package main

import (
	"maps"
	"testing"

	"github.com/stretchr/testify/assert"
)

func TestClonedMapsAreNotSame(t *testing.T) {
	original := map[string]int{
		"a": 1,
		"b": 2,
		"c": 3,
	}

	cloned := maps.Clone(original)

	assert.NotSame(t, original, cloned)
}

func TestSameMaps(t *testing.T) {
	original := map[string]int{
		"a": 1,
		"b": 2,
		"c": 3,
	}

	cloned := original
	assert.Same(t, original, cloned)

	cloned["d"] = 4
	assert.Same(t, original, cloned)
}
```

which fails with the following after the original dependency update:
```
--- FAIL: TestClonedMapsAreNotSame (0.00s)
    main_test.go:19:
                Error Trace:    /Users/jesse/tmp/testify/main_test.go:19
                Error:          Both arguments must be pointers
                Test:           TestClonedMapsAreNotSame
--- FAIL: TestSameMaps (0.00s)
    main_test.go:30:
                Error Trace:    /Users/jesse/tmp/testify/main_test.go:30
                Error:          Both arguments must be pointers
                Test:           TestSameMaps
    main_test.go:33:
                Error Trace:    /Users/jesse/tmp/testify/main_test.go:33
                Error:          Both arguments must be pointers
                Test:           TestSameMaps
FAIL
FAIL    testassertequal 0.149s
FAIL
```

However, instead of fixing the underlying issue, we took the address of those variables and kept using `assert.Same`. This isn't meaningful, since it doesn't directly compare the underlying pointers of the map objects in question, just the address of the pointers to those maps. Here's the output after taking the address (e.g. `&original` and `&cloned`):

```
--- FAIL: TestSameMaps (0.00s)
    main_test.go:30:
                Error Trace:    /Users/jesse/tmp/testify/main_test.go:30
                Error:          Not same:
                                expected: 0x14000070170 &map[string]int{"a":1, "b":2, "c":3}
                                actual  : 0x14000070178 &map[string]int{"a":1, "b":2, "c":3}
                Test:           TestSameMaps
    main_test.go:33:
                Error Trace:    /Users/jesse/tmp/testify/main_test.go:33
                Error:          Not same:
                                expected: 0x14000070170 &map[string]int{"a":1, "b":2, "c":3, "d":4}
                                actual  : 0x14000070178 &map[string]int{"a":1, "b":2, "c":3, "d":4}
                Test:           TestSameMaps
FAIL
FAIL    testassertequal 0.157s
FAIL
```

They are obviously the same map, since modifying `cloned` modified the
original, yet `assert.Same` thinks they are different (because the
pointe values are indeed different). (`assert.NotSame` "passes", but for
the wrong reasons.)

To fix this, introduce `model.AssertNotSameMap` to check this correctly.
2025-10-27 13:16:59 -03:00
Ben Schumacher
9add320011
[MM-64654] Migrate to modern Go features (#31820) 2025-07-18 12:54:51 +02:00
Agniva De Sarker
c4718e4542
MM-60211: Bump dependencies (#30005)
https://mattermost.atlassian.net/browse/MM-60211
```release-note
NONE
```

Co-authored-by: Mattermost Build <build@mattermost.com>
2025-02-06 21:48:08 +05:30
Ben Schumacher
51e73b681b
[MM-40638] Type all of the websocket event names as a new type WebsocketEventType (#25454)
Co-authored-by: Sudheer Tripathi <sudheer@clearglass.com>
Co-authored-by: Sudheer Tripathi <31629433+sudheer121@users.noreply.github.com>
Co-authored-by: Sudheer Tripathi <tripathisudheer604@gmail.com>
2023-11-22 11:09:48 +01:00
Agniva De Sarker
3563e56a77
MM-54998: Optimize JSON marshalling in websocket broadcast (#25286)
Marshalling a json.RawMessage is not zero overhead. Instead,
it compacts the raw message which starts to have an overhead
at scale.

https://github.com/golang/go/issues/33422

Since we have full control over the message constructed, we
can simply write the byte slice into the network stream.
This gives considerable performance boost.

```
goos: linux
goarch: amd64
pkg: github.com/mattermost/mattermost/server/public/model
cpu: Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz
             │   old.txt    │              new_2.txt              │
             │    sec/op    │   sec/op     vs base                │
EncodeJSON-8   1640.5n ± 2%   289.6n ± 1%  -82.35% (p=0.000 n=10)

             │  old.txt   │             new_2.txt             │
             │    B/op    │    B/op     vs base               │
EncodeJSON-8   528.0 ± 0%   503.0 ± 0%  -4.73% (p=0.000 n=10)

             │  old.txt   │             new_2.txt              │
             │ allocs/op  │ allocs/op   vs base                │
EncodeJSON-8   5.000 ± 0%   4.000 ± 0%  -20.00% (p=0.000 n=10)
```

P.S. No concerns over changing the model API because we are
still using 0.x

https://mattermost.atlassian.net/browse/MM-54998

```release-note
Improve websocket event marshalling performance
```
2023-11-08 12:15:24 +05:30
Agniva De Sarker
d9a28c1244
MM-52532: Fix golangci warnings for public module (#23918)
https://mattermost.atlassian.net/browse/MM-52532

```release-note
NONE
```
2023-06-30 20:12:56 +05:30
Agniva De Sarker
c249ba4a66
MM-52532: Fix golangci warnings (#23709)
https://mattermost.atlassian.net/browse/MM-52532

- Replace golint with revive
- Add makezero linter
- Fix all the required linter failures

Some issues in enterprise and public modules
are yet to be fixed. We send this to expediate things.
2023-06-13 14:08:36 +05:30
Jesse Hallam
bb02b35048
Expose public/ API as submodule (#23345)
* model -> public/model

* plugin -> public/plugin

* public/model/utils -> public/utils

* platform/shared/mlog -> public/shared/mlog

* platform/shared/i18n -> public/shared/i18n

* platform/shared/markdown -> public/shared/markdown

* platform/services/timezones -> public/shared/timezones

* channels/einterfaces -> einterfaces

* expose public/ submodule

* go mod tidy

* .github: cache-dependency-path, setup-go-work

* modules-tidy for public/ too

* remove old gomodtidy
2023-05-10 13:07:02 -03:00
Renamed from server/model/websocket_message_test.go (Browse further)