Commit graph

3 commits

Author SHA1 Message Date
Kamron Batman
9a25054809
test: remove trivial, duplicate and content-value tests (#2721)
## Summary
Many tests proved a specific change when it landed but don't need to run on every CI build forever. This removes **202 test methods and 9 files: 313 test cases, 2,247 → 1,934**. UOContent.Tests drops from about 14s to about 5s locally.

Removed:
- **Tautologies.** Asserting a constant, a constructor readback or a trivial getter, or testing BCL behavior.
- **Exact duplicates** of a surviving test on the same code path.
- **Obsolete regressions** whose failure can no longer be expressed.
- **Game-content values** shard owners customize (era drop rates, loot and skill tables, single-creature tuning). Each customization would otherwise force a test fix.

## How it was decided
1. Every test was classified, with a reason, by a per-shard sweep over all 287 test files.
2. Each proposed deletion was then checked by a second, adversarial reviewer against the surviving tests and the production code.
3. Anything that was the only coverage of a separate code path was **kept**, even where it looked like a duplicate. Examples: the unsigned `SpanReader`/`SpanWriter` methods, the at-location versus bounds enumerators, null-map enumerables, the security filters, archive-journal state transitions. About 130 proposed deletions were rejected this way.

Whole files removed:
- `FishingTests`, apart from 3 tests: the map-edge scan, serpent cleanup, and SOS chest consumption. The SOS test now stops at the first chest instead of measuring a catch rate. The multi-million-cast rate simulations are gone.
- `LordOaksTests` and `MeerEnragedTests` (single-creature tests).
- `AutoArchiveHelperTests`, `JsonDiscoverableTypeAttributeTests`, `DecimalSerializationTests` and `EnumConversionTests` (tautologies, or tests of their own local helpers).
- `NetworkStatsTests`.
- `MigratedDataLoadTests` (a subset of `AllSpawnFilesLoadTests`).
- `SpawnerFixtureCapture`: a capture tool, not a test, and its `version == 12` assert can no longer pass. The `.bin` fixtures it produced stay.

## Tests
- Server.Tests: 811 pass.
- UOContent.Tests: 1,123 pass.

A follow-up PR, stacked on this one, fixes how the remaining tests behave: shared state, CI-vacuous tests, harness duplication.
2026-10-18 20:08:19 -07:00
Kamron Batman
be3a08513f
fix: Fixes PlayerConstructed stacking/BODs (#2579)
### Summary

* Removes player constructed as a requirement for BODs.
* When two items stack and they don't match player constructed flags, the resulting stack loses the flag.
2026-08-14 17:14:38 -07:00
Kamron Batman
9b35b39d0d
fix: stop stack merges and splits from laundering PlayerConstructed (#2576)
## Why

`PlayerConstructed` is per-instance provenance, and #2574 put it on every crafted item — including potions, arrows and other stackables. Stack operations were written when no item carried provenance of any kind, so they treated two piles of the same graphic as interchangeable.

**Merging** keeps the receiving stack's value. Dropping bought potions onto a crafted stack made the whole pile count as crafted; the reverse order erased it. Which one happened was decided by drag direction alone.

**Splitting** rebuilds one half in `Mobile.LiftItemDupe`, which copies a fixed list of fields rather than going through `Dupe`/`CopyProperties`. `PlayerConstructed` was not on that list, so dragging part of a pile off stripped the new half. Worth calling out: `[IgnoreDupe]` does **not** govern this path — it only applies to `Dupe()`. Reasoning "the field isn't `[IgnoreDupe]`, so it copies" is wrong here.

## Changes

- `Item.CanStackWith` compares `PlayerConstructed`, so crafted and non-crafted never merge into one indistinguishable pile.
- `Mobile.LiftItemDupe` copies `PlayerConstructed` onto the remainder, so a split cannot produce halves that disagree about what they are.

Refusing to merge is the whole fix. A stack has nowhere to record provenance, so the only coherent behaviour is to keep the two piles apart rather than pick a winner.

## What this deliberately does not do

Paths that genuinely **virtualize** an item — pouring from a `PotionKeg`, for one — rebuild it without the flag, and the result is simply treated as not crafted. That is accepted rather than worked around; the alternative is threading provenance through every count-based container, which buys little. The keg stores a `Held` int rather than a stack, so nothing there depends on merging and nothing breaks.

`CommodityDeed` is unaffected — it holds the real `Commodity` item rather than a count, so the flag rides along.

## Player-visible effect

Crafted potions and arrows will no longer stack with bought or looted ones. That is the intended invariant, and it is the reason the flag can be trusted at all.

## Tests

7 new tests in `Server.Tests`: both merge directions, the matching-provenance case, split copying, and the split/re-merge round trip.

`Server.Tests` **822 passing**, `UOContent.Tests` **701 passing**, build clean with 0 warnings.
2026-08-13 19:18:06 -07:00