search

zond / godip

A dippy adjudicator in Go.
GNU General Public License v3.0
27 stars 22 forks source link

Options testing for full games is broken #49

Closed tttppp closed 5 years ago

tttppp commented 5 years ago

I noticed that using BENCHMARK_OPTIONS=true causes validation of the options, which are actually computed incorrectly (before this change). For some reason the tests seem to think that a unit can move via convoy to the same spot, and an army can move via convoy to a sea space. Also it's suggesting builds in non-SCs. This is never suggested in a real game, so I assume it's an issue with the test framework. I'll raise a separate bug for this.

As an example here's the output for the first Fall and Adjustment of test game 1 for the Hundred variant:

.../godip/variants/hundred$ env BENCHMARK_OPTIONS=true DEBUG=true go test -v ./...
=== RUN   TestGames
Testing Hundred game_1.txt
Checked 17 phases, executed 139 orders and asserted 525 positions in game_1.txt, found 0 failures.
Spent on average 605.497µs calculating options, never more than 2.011284ms.--- FAIL: TestGames (0.07s)
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order cal Move cal: ErrIllegalMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [cal eng]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [cal str]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [fla eng]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [fla str]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [brt str]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [brt eng]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [nom eng]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order lon Support [nom str]: ErrIllegalSupportMove
    games_testing.go:148: Wanted "England", got ""
    games_testing.go:151: got invalid order brt Move brt: ErrIllegalMove
    games_testing.go:148: Wanted "Burgundy", got ""
    games_testing.go:151: got invalid order fla Move fla: ErrIllegalMove
    games_testing.go:148: Wanted "France", got ""
    games_testing.go:151: got invalid order nom Move nom: ErrIllegalMove
    games_testing.go:148: Wanted "Burgundy", got ""
    games_testing.go:151: got invalid order Army Build fla: ErrMissingSupplyCenter
    games_testing.go:148: Wanted "Burgundy", got ""
    games_testing.go:151: got invalid order Fleet Build fla: ErrMissingSupplyCenter
    games_testing.go:148: Wanted "Burgundy", got ""
    games_testing.go:151: got invalid order Army Build hol: ErrMissingSupplyCenter
    games_testing.go:148: Wanted "Burgundy", got ""
    games_testing.go:151: got invalid order Fleet Build hol: ErrMissingSupplyCenter
    games_testing.go:148: Wanted "Burgundy", got ""
    games_testing.go:151: got invalid order Army Build lux: ErrMissingSupplyCenter
...
=== RUN   TestLondonCalais
--- PASS: TestLondonCalais (0.00s)
=== RUN   TestBuildAnywhere
--- PASS: TestBuildAnywhere (0.00s)
=== RUN   TestConvoy
--- PASS: TestConvoy (0.00s)
=== RUN   TestDisbandFrenchUnit
--- PASS: TestDisbandFrenchUnit (0.00s)
FAIL
FAIL    github.com/zond/godip/variants/hundred  0.077s

This is while testing this position: https://www.vdiplomacy.com/map.php?gameID=33990&turn=0&mapType=large

tttppp commented 5 years ago

I'm working on this at the moment

tttppp commented 5 years ago

The changes are on this branch: https://github.com/zond/godip/tree/OptionFixes

Currently just one commit here: https://github.com/zond/godip/commit/0754cb0c824f382eee09594375e7a0785e38af95

All the 'normal' tests are passing including options testing for full games using BENCHMARK_OPTIONS=true. I'm currently testing the droidippy games, but I'm expecting this to take about an hour to run through.

Still to do:

  1. Compare the benchmark duration to check performance hasn't been impacted (probably using the Youngstown games will be sufficient for this).
  2. Find out when the regression happened (not really essential, but I'm interested).
tttppp commented 5 years ago

Two games from the droidippy set give me panics (game_2742.txt and game_2879.txt):

$ env SKIP=game_2879.txt go test -v ./...
=== RUN   TestDroidippyGames
Testing Classical game_2879.txt
--- FAIL: TestDroidippyGames (0.02s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x5279f9]

goroutine 5 [running]:
testing.tRunner.func1(0xc00011a200)
    /snap/go/current/src/testing/testing.go:830 +0x392
panic(0x57a180, 0x702110)
    /snap/go/current/src/runtime/panic.go:522 +0x1b5
github.com/zond/godip/state.(*resolver).adjudicate(0xc00029a450, 0xc000450cf9, 0x6, 0x6, 0xc00028db69)
    ...go/src/github.com/zond/godip/state/resolver.go:19 +0x149
github.com/zond/godip/state.(*resolver).Resolve(0xc00029a450, 0xc000450cf9, 0x6, 0x3, 0x1)
    ...go/src/github.com/zond/godip/state/resolver.go:43 +0x95e
github.com/zond/godip/orders.MoveSupport.func1(0xc000450cf9, 0x6, 0x5e0fc0, 0xc000389fa0, 0xc0002ac820, 0xc000450cf9)
    ...go/src/github.com/zond/godip/orders/move.go:358 +0x151
github.com/zond/godip/state.(*State).Find(0xc0001e5040, 0xc0000b7310, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
    ...go/src/github.com/zond/godip/state/state.go:145 +0x39c
github.com/zond/godip/orders.MoveSupport(0x5e1c60, 0xc00029a450, 0xc000451f30, 0x3, 0xc000451f39, 0x3, 0xc0002ae590, 0x1, 0x1, 0x702201)
    ...go/src/github.com/zond/godip/orders/move.go:350 +0xde
github.com/zond/godip/orders.(*move).adjudicateMovementPhase(0xc000389e20, 0x5e1c60, 0xc00029a450, 0x56b100, 0xc000403710)
    ...go/src/github.com/zond/godip/orders/move.go:152 +0x2dc
github.com/zond/godip/orders.(*move).Adjudicate(0xc000389e20, 0x5e1c60, 0xc00029a450, 0x3, 0xc0004037c8)
    ...go/src/github.com/zond/godip/orders/move.go:64 +0xc7
github.com/zond/godip/state.(*resolver).adjudicate(0xc00029a450, 0xc000451f30, 0x3, 0x3, 0xc00028db68)
    ...go/src/github.com/zond/godip/state/resolver.go:19 +0x16c
github.com/zond/godip/state.(*resolver).Resolve(0xc00029a450, 0xc000451f30, 0x3, 0x3, 0xc00044ec08)
    ...go/src/github.com/zond/godip/state/resolver.go:43 +0x95e
github.com/zond/godip/state.(*State).Next(0xc0001e5040, 0x5aaf0d, 0x6)
    ...go/src/github.com/zond/godip/state/state.go:209 +0x78a
github.com/zond/godip/variants/testing.setPhase(0xc00011a200, 0xc000221c20, 0xc0001e66c0, 0x4, 0x4, 0x5b5460)
    ...go/src/github.com/zond/godip/variants/testing/games_testing.go:129 +0x19f
github.com/zond/godip/variants/testing.assertGame(0xc00011a200, 0xc0001c19c0, 0xd, 0x7065c0, 0x7, 0x7, 0x5b5470, 0x5b5460, 0xc000221d58, 0x0, ...)
    ...go/src/github.com/zond/godip/variants/testing/games_testing.go:251 +0x1b11
github.com/zond/godip/variants/testing.TestGames(0xc00011a200, 0x5ab694, 0x9, 0x5b5470, 0x5b5498, 0x5b5460, 0x5b5468, 0xc0000b0000, 0x7, 0x7, ...)
    ...go/src/github.com/zond/godip/variants/testing/games_testing.go:278 +0x4c8
github.com/zond/godip/variants/classical/droidippy.TestDroidippyGames(0xc00011a200)
    ...go/src/github.com/zond/godip/variants/classical/droidippy/droidippy_test.go:17 +0x6a
testing.tRunner(0xc00011a200, 0x5b54f8)
    /snap/go/current/src/testing/testing.go:865 +0xc0
created by testing.(*T).Run
    /snap/go/current/src/testing/testing.go:916 +0x35a
FAIL    github.com/zond/godip/variants/classical/droidippy  0.024s

I vaguely remember that this has always happened for me - I don't think it's anything to do with these changes.

tttppp commented 5 years ago

All the Droidippy tests passed (except for 2742 and 2879).

I had a look at the panic for 2742 and it seems to be due to:

PHASE 1910 Fall Movement
POSITIONS
...
Italy: fleet bul/sc
...
ORDERS
...
bul support aeg move con

(rather than bul/sc support aeg move con)

tttppp commented 5 years ago

Benchmarking turns out to not be very fair. Because there were so many errors before then the test has to store a massive list of them all in memory. Anyway it seems very quick to run all the tests now.