Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions docs/CODING_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,9 @@ Refer to the [ADD_NEW_EXCHANGE.md](/docs/ADD_NEW_EXCHANGE.md) document for compr
### Type Usage

- Use the most appropriate native Go types for struct fields:
- If the API returns numbers as strings, use float64 with the `json:",string"` tag.
- For timestamps, use `time.Time` if Go's JSON unmarshalling supports the format directly.
- If native Go types are not supported directly, use the following built-in types:
- `types.Time` for Unix timestamps that require custom unmarshalling.
- `types.Number` for numerical float values where an exchange API may return either a `string` or `float64` value.
- If the API always returns a number as a bare JSON number, use `float64`.
- If the API returns a number as a quoted JSON string, or may return either a string or a bare number, use `types.Number` — **do not** use `float64` with the `json:",string"` tag.
- For timestamps, use `time.Time` if Go's JSON unmarshalling supports the format directly; otherwise use `types.Time` for Unix timestamps that require custom unmarshalling.
- Always use full and descriptive field names for clarity and consistency. Avoid short API-provided aliases unless compatibility requires it.
- Default to `uint64` for exchange API parameters and structs for integers where appropriate.
- Avoid `int` (size varies by architecture) or `int64` (allows negatives where they don't make sense).
Expand Down Expand Up @@ -112,13 +110,27 @@ Use `require` and `assert` appropriately:

- Use when test flow depends on the result.
- Messages must contain **"must"** (e.g., "response must not be nil").
- Use the *f* variants when using format specifiers (e.g., `require.Equalf`).

#### assert

- Use when the test can proceed regardless of the check.
- Messages must contain **"should"** (e.g., "status code should be 200").
- Use `assert.Equalf`, etc., when applicable.

#### `f` variants (`assert.ErrorIsf`, `require.NoErrorf`, etc.)

- Only use `f` variants when the message contains **format verbs** (e.g., `%s`, `%d`, `%v`).
- If the message is a plain string with no format verbs, use the non-`f` variant.

```go
// Correct — format verb %s requires the f variant:
assert.NoErrorf(t, err, "UpdateAccountInfo should not error for asset %s", a)

// Correct — plain message, no format verbs:
assert.ErrorIs(t, err, errInvalidOrderSize, "validate should return expected error")

// Wrong — f variant used without format verbs:
assert.ErrorIsf(t, err, errInvalidOrderSize, "validate should return expected error")
```

### Test Coverage

Expand Down
27 changes: 16 additions & 11 deletions engine/rpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,14 @@ const (
fakeExchangeName = "fake"
)

var errExpectedTestError = errors.New("expected test error")
var (
errExpectedTestError = errors.New("expected test error")
testExchangeCounter common.Counter
)

func newUniqueFakeExchangeName() string {
return fmt.Sprintf("%s-%d", fakeExchangeName, testExchangeCounter.IncrementAndGet())
}

// fExchange is a fake exchange with function overrides
// we're not testing an actual exchange's implemented functions
Expand Down Expand Up @@ -3141,7 +3148,8 @@ func TestGetOrderbookAmountByNominal(t *testing.T) {

exch.SetDefaults()
b := exch.GetBase()
b.Name = fakeExchangeName
uniqueFakeExchangeName := newUniqueFakeExchangeName()
b.Name = uniqueFakeExchangeName
b.Enabled = true

cp := currency.NewPairWithDelimiter("btc", "meme", "-")
Expand All @@ -3167,7 +3175,7 @@ func TestGetOrderbookAmountByNominal(t *testing.T) {
_, err = s.GetOrderbookAmountByNominal(t.Context(), req)
require.ErrorIs(t, err, common.ErrExchangeNameNotSet)

req.Exchange = "fake"
req.Exchange = uniqueFakeExchangeName
_, err = s.GetOrderbookAmountByNominal(t.Context(), req)
require.ErrorIs(t, err, asset.ErrNotSupported)

Expand All @@ -3181,9 +3189,7 @@ func TestGetOrderbookAmountByNominal(t *testing.T) {
Quote: currency.MEME.String(),
}
_, err = s.GetOrderbookAmountByNominal(t.Context(), req)
if !strings.Contains(err.Error(), "cannot find orderbook") {
t.Fatalf("received: '%+v' but expected: '%v'", err, "cannot find orderbook")
}
require.ErrorIs(t, err, orderbook.ErrOrderbookNotFound)

depth, err := orderbook.DeployDepth(req.Exchange, currency.NewPair(currency.BTC, currency.MEME), asset.Spot)
require.NoError(t, err, "orderbook.DeployDepth must not error")
Expand Down Expand Up @@ -3227,7 +3233,8 @@ func TestGetOrderbookAmountByImpact(t *testing.T) {

exch.SetDefaults()
b := exch.GetBase()
b.Name = fakeExchangeName
uniqueFakeExchangeName := newUniqueFakeExchangeName()
b.Name = uniqueFakeExchangeName
b.Enabled = true

cp := currency.NewPairWithDelimiter("btc", "mad", "-")
Expand All @@ -3253,7 +3260,7 @@ func TestGetOrderbookAmountByImpact(t *testing.T) {
_, err = s.GetOrderbookAmountByImpact(t.Context(), req)
require.ErrorIs(t, err, common.ErrExchangeNameNotSet)

req.Exchange = "fake"
req.Exchange = uniqueFakeExchangeName
_, err = s.GetOrderbookAmountByImpact(t.Context(), req)
require.ErrorIs(t, err, asset.ErrNotSupported)

Expand All @@ -3267,9 +3274,7 @@ func TestGetOrderbookAmountByImpact(t *testing.T) {
Quote: currency.MAD.String(),
}
_, err = s.GetOrderbookAmountByImpact(t.Context(), req)
if !strings.Contains(err.Error(), "cannot find orderbook") {
t.Fatalf("received: '%+v' but expected: '%v'", err, "cannot find orderbook")
}
require.ErrorIs(t, err, orderbook.ErrOrderbookNotFound)

depth, err := orderbook.DeployDepth(req.Exchange, currency.NewPair(currency.BTC, currency.MAD), asset.Spot)
require.NoError(t, err, "orderbook.DeployDepth must not error")
Expand Down
Loading
Loading