From c4d87b59d6df887c4bf4e01e689f6d1b01544990 Mon Sep 17 00:00:00 2001 From: "alexei.dolgolyov" Date: Tue, 5 May 2026 12:09:44 +0300 Subject: [PATCH] =?UTF-8?q?fix(initial-implementation):=20close=20P2/P3/P5?= =?UTF-8?q?=20review=20blockers=20=E2=80=94=20185/185=20tests=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Combined-batch reviewer flagged three real blockers + two test-infra issues across the parallel P2/P3/P5 batch. All resolved: PHASE 3 — DateTimeOffset UTC-kind constructor (3 sites) EventListingParserBase.cs:39, EventOddsParser.cs:72, ResultsParser.cs:104 Replaced `new DateTimeOffset(DateTimeOffset.UtcNow.UtcDateTime, MoscowOffset)` (throws ArgumentException because UtcDateTime has Kind=Utc) with `DateTimeOffset.UtcNow.ToOffset(MoscowOffset)`. PHASE 2 — EF string.Compare not translatable (3 sites) EventRepository.cs:34, SnapshotRepository.cs:46, ExcelExporter.cs:35 Replaced `string.Compare(col, str, StringComparison.Ordinal)` with `col.CompareTo(str)` so EF Core's SQLite provider can translate the expression. Semantics unchanged (SQLite default collation = BINARY = ordinal). PHASE 3 — ServerTimeProvider regex misses JSON-quoted key Regex `serverTime\s*:\s*"..."` only matched bare-key form. Updated to `"?serverTime"?\s*:\s*"..."` so the JSON-quoted form (the actual marathonbet.by production format) is matched. PHASE 3 — fixture: orphan elements stripped by HTML5 parser tests/.../Fixtures/marathonbet/event-football-sample.html — wrapped the blocks in a proper hierarchy so AngleSharp preserves them and `td.Closest("td")` succeeds in the parser. PHASE 2 — InMemoryDbFixture shared state across parallel tests All fixture instances used `Data Source=marathon_tests` causing xUnit's parallel-within-class runs to contaminate each other's data. Each fixture now uses a Guid-suffixed unique data source name. PLAN.md — P2/P3/P5 rows updated to ✅ Done with batch commit reference. Test status: Domain.Tests: 96/96 ✅ Application.Tests: 1/1 ✅ Infrastructure.Tests: 77/77 ✅ UI.Tests: 11/11 ✅ TOTAL: 185/185 ✅ Build: 0 warnings, 0 errors. Deferred to later phases (per reviewer 🟡 / 🔵 notes): - SnapshotRepository.GetAsync(Guid) uses lossy GetHashCode workaround; Phase 4 to fix or remove from interface. - Excel Sport name column writes string.Empty (need lookup join in Phase 6). - PeriodScopeMapper football n>2 falls through to "Quarter" token; guarded by MaxPeriods today, but defensive cleanup at Phase 9. - Settings.razor duplicate m-rise-5 class on Localization section. --- plans/initial-implementation/PLAN.md | 6 +- .../Export/ExcelExporter.cs | 4 +- .../Repositories/EventRepository.cs | 7 +- .../Repositories/SnapshotRepository.cs | 4 +- .../Parsers/EventListingParserBase.cs | 2 +- .../Scraping/Parsers/EventOddsParser.cs | 2 +- .../Scraping/Parsers/ResultsParser.cs | 2 +- .../Scraping/Parsers/ServerTimeProvider.cs | 6 +- .../marathonbet/event-football-sample.html | 90 ++++++++++--------- .../Persistence/InMemoryDbFixture.cs | 14 ++- 10 files changed, 79 insertions(+), 58 deletions(-) diff --git a/plans/initial-implementation/PLAN.md b/plans/initial-implementation/PLAN.md index ccdc1ff..b01217d 100644 --- a/plans/initial-implementation/PLAN.md +++ b/plans/initial-implementation/PLAN.md @@ -64,10 +64,10 @@ parameter configurable. |---|---|---|---|---|---| | Phase 0: Scraping spike | backend | ✅ Done | ⚠️ Pass with notes (Sonnet) | ⏭️ N/A (research) | ✅ 070e34b | | Phase 1: Solution + Domain | backend | ✅ Done | ⚠️ Pass with notes (Sonnet) | ✅ Build OK + 96/96 Domain tests | ✅ 61114ea | -| Phase 2: Storage | backend | 🔨 Code done, not committed | ⬜ Pending | ⏭️ Big Bang (own code 0/0) | ⬜ WIP | -| Phase 3: Scraping | backend | 🔨 Code done, not committed | ⬜ Pending | ⏭️ Big Bang (own code 0/0) | ⬜ WIP | +| Phase 2: Storage | backend | ✅ Done | ⚠️ Pass with notes (Sonnet, combined batch) | ✅ Build OK + 77/77 Infra tests | ✅ batch (e4d8476…686550d…+) | +| Phase 3: Scraping | backend | ✅ Done | ⚠️ Pass with notes (Sonnet, combined batch) | ✅ Build OK + 77/77 Infra tests | ✅ batch (e4d8476…686550d…+) | | Phase 4: Application + Workers | backend | ⬜ Not Started | ⬜ | ⏭️ Big Bang | ⬜ | -| Phase 5: Host + Theme + i18n | frontend | 🔨 ~95% (killed mid-final-verify) | ⬜ Pending | ⏭️ Big Bang | ⬜ WIP | +| Phase 5: Host + Theme + i18n | frontend | ✅ Done | ⚠️ Pass with notes (Sonnet, combined batch) | ✅ Build OK + 11/11 UI tests | ✅ batch (e4d8476…686550d…+) | | Phase 6: Event browsing UI | frontend | ⬜ Not Started | ⬜ | ⏭️ Big Bang | ⬜ | | Phase 7: Anomaly detection | fullstack | ⬜ Not Started | ⬜ | ⏭️ Big Bang | ⬜ | | Phase 8: Results loader | fullstack | ⬜ Not Started | ⬜ | ⏭️ Big Bang | ⬜ | diff --git a/src/Marathon.Infrastructure/Export/ExcelExporter.cs b/src/Marathon.Infrastructure/Export/ExcelExporter.cs index ff3f313..39b5ef5 100644 --- a/src/Marathon.Infrastructure/Export/ExcelExporter.cs +++ b/src/Marathon.Infrastructure/Export/ExcelExporter.cs @@ -32,8 +32,8 @@ internal sealed class ExcelExporter : IExcelExporter var snapshotEntities = await _db.Snapshots.AsNoTracking() .Include(s => s.Bets) .Include(s => s.Event) - .Where(s => string.Compare(s.CapturedAt, fromStr, StringComparison.Ordinal) >= 0 - && string.Compare(s.CapturedAt, toStr, StringComparison.Ordinal) <= 0) + .Where(s => s.CapturedAt.CompareTo(fromStr) >= 0 + && s.CapturedAt.CompareTo(toStr) <= 0) .ToListAsync(ct); // Convert to domain objects for processing diff --git a/src/Marathon.Infrastructure/Persistence/Repositories/EventRepository.cs b/src/Marathon.Infrastructure/Persistence/Repositories/EventRepository.cs index 5ecf349..dfb6794 100644 --- a/src/Marathon.Infrastructure/Persistence/Repositories/EventRepository.cs +++ b/src/Marathon.Infrastructure/Persistence/Repositories/EventRepository.cs @@ -30,9 +30,12 @@ internal sealed class EventRepository : IEventRepository var fromStr = range.From.ToString("O"); var toStr = range.To.ToString("O"); + // EF Core SQLite cannot translate string.Compare(...) with StringComparison; it can + // translate the relational operators on string columns (which use BINARY/ordinal + // collation by default in SQLite — correct for ISO 8601). var entities = await _db.Events.AsNoTracking() - .Where(e => string.Compare(e.ScheduledAt, fromStr, StringComparison.Ordinal) >= 0 - && string.Compare(e.ScheduledAt, toStr, StringComparison.Ordinal) <= 0) + .Where(e => e.ScheduledAt.CompareTo(fromStr) >= 0 + && e.ScheduledAt.CompareTo(toStr) <= 0) .ToListAsync(ct); return entities.Select(Mapping.ToDomain).ToList().AsReadOnly(); diff --git a/src/Marathon.Infrastructure/Persistence/Repositories/SnapshotRepository.cs b/src/Marathon.Infrastructure/Persistence/Repositories/SnapshotRepository.cs index f7195e6..f9dbb2a 100644 --- a/src/Marathon.Infrastructure/Persistence/Repositories/SnapshotRepository.cs +++ b/src/Marathon.Infrastructure/Persistence/Repositories/SnapshotRepository.cs @@ -43,8 +43,8 @@ internal sealed class SnapshotRepository : ISnapshotRepository var entities = await _db.Snapshots.AsNoTracking() .Include(s => s.Bets) .Where(s => s.EventCode == eventId.Value - && string.Compare(s.CapturedAt, fromStr, StringComparison.Ordinal) >= 0 - && string.Compare(s.CapturedAt, toStr, StringComparison.Ordinal) <= 0) + && s.CapturedAt.CompareTo(fromStr) >= 0 + && s.CapturedAt.CompareTo(toStr) <= 0) .ToListAsync(ct); return entities.Select(Mapping.ToDomain).ToList().AsReadOnly(); diff --git a/src/Marathon.Infrastructure/Scraping/Parsers/EventListingParserBase.cs b/src/Marathon.Infrastructure/Scraping/Parsers/EventListingParserBase.cs index a7bc47e..6e0ca95 100644 --- a/src/Marathon.Infrastructure/Scraping/Parsers/EventListingParserBase.cs +++ b/src/Marathon.Infrastructure/Scraping/Parsers/EventListingParserBase.cs @@ -36,7 +36,7 @@ public abstract class EventListingParserBase CancellationToken ct) { var serverTime = ServerTimeProvider.ExtractServerTime(html) - ?? new DateTimeOffset(DateTimeOffset.UtcNow.UtcDateTime, MoscowOffset); + ?? DateTimeOffset.UtcNow.ToOffset(MoscowOffset); var config = AngleSharpConfig.Default; using var context = BrowsingContext.New(config); diff --git a/src/Marathon.Infrastructure/Scraping/Parsers/EventOddsParser.cs b/src/Marathon.Infrastructure/Scraping/Parsers/EventOddsParser.cs index fc353c3..580c046 100644 --- a/src/Marathon.Infrastructure/Scraping/Parsers/EventOddsParser.cs +++ b/src/Marathon.Infrastructure/Scraping/Parsers/EventOddsParser.cs @@ -69,7 +69,7 @@ public sealed partial class EventOddsParser : IEventOddsParser ArgumentNullException.ThrowIfNull(html); var capturedAt = _serverTime.ExtractServerTime(html) - ?? new DateTimeOffset(DateTimeOffset.UtcNow.UtcDateTime, MoscowOffset); + ?? DateTimeOffset.UtcNow.ToOffset(MoscowOffset); var config = AngleSharpConfig.Default; using var context = BrowsingContext.New(config); diff --git a/src/Marathon.Infrastructure/Scraping/Parsers/ResultsParser.cs b/src/Marathon.Infrastructure/Scraping/Parsers/ResultsParser.cs index 610e71b..1961bc4 100644 --- a/src/Marathon.Infrastructure/Scraping/Parsers/ResultsParser.cs +++ b/src/Marathon.Infrastructure/Scraping/Parsers/ResultsParser.cs @@ -101,7 +101,7 @@ public sealed partial class ResultsParser : IResultsParser if (string.IsNullOrWhiteSpace(eventIdRaw)) return null; - var completedAt = new DateTimeOffset(DateTimeOffset.UtcNow.UtcDateTime, MoscowOffset); + var completedAt = DateTimeOffset.UtcNow.ToOffset(MoscowOffset); return new EventResult( new DomainEventId(eventIdRaw), diff --git a/src/Marathon.Infrastructure/Scraping/Parsers/ServerTimeProvider.cs b/src/Marathon.Infrastructure/Scraping/Parsers/ServerTimeProvider.cs index a2ffa5d..6006086 100644 --- a/src/Marathon.Infrastructure/Scraping/Parsers/ServerTimeProvider.cs +++ b/src/Marathon.Infrastructure/Scraping/Parsers/ServerTimeProvider.cs @@ -9,9 +9,11 @@ namespace Marathon.Infrastructure.Scraping.Parsers; /// public sealed partial class ServerTimeProvider : IServerTimeProvider { - // Matches: serverTime:"YYYY,MM,DD,HH,mm,ss" + // Matches both forms observed on marathonbet.by: + // serverTime:"YYYY,MM,DD,HH,mm,ss" (bare key) + // "serverTime":"YYYY,MM,DD,HH,mm,ss" (JSON-quoted key — actual prod format) [GeneratedRegex( - @"serverTime\s*:\s*""(\d{4}),(\d{1,2}),(\d{1,2}),(\d{1,2}),(\d{1,2}),(\d{1,2})""", + @"""?serverTime""?\s*:\s*""(\d{4}),(\d{1,2}),(\d{1,2}),(\d{1,2}),(\d{1,2}),(\d{1,2})""", RegexOptions.CultureInvariant)] private static partial Regex ServerTimeRegex(); diff --git a/tests/Marathon.Infrastructure.Tests/Fixtures/marathonbet/event-football-sample.html b/tests/Marathon.Infrastructure.Tests/Fixtures/marathonbet/event-football-sample.html index dd1331f..0b017c3 100644 --- a/tests/Marathon.Infrastructure.Tests/Fixtures/marathonbet/event-football-sample.html +++ b/tests/Marathon.Infrastructure.Tests/Fixtures/marathonbet/event-football-sample.html @@ -29,46 +29,56 @@
- - - 1.65 - - - 4.10 - - - 5.70 - - - - - (-1.0)
- 2.04 - - - (+1.0)
- 1.82 - - - - - (2.5)
- 1.92 - - - (2.5)
- 1.92 - - - -1.80 -3.60 -4.20 - - -2.10 -2.80 -3.50 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ 1.65 + + 4.10 + + 5.70 +
+ (-1.0)
+ 2.04 + 		+ (+1.0)
+ 1.82 +
+ (2.5)
+ 1.92 + 		+ (2.5)
+ 1.92 +
1.803.604.20
2.102.803.50
diff --git a/tests/Marathon.Infrastructure.Tests/Persistence/InMemoryDbFixture.cs b/tests/Marathon.Infrastructure.Tests/Persistence/InMemoryDbFixture.cs index 4b4e1cb..c0c1e25 100644 --- a/tests/Marathon.Infrastructure.Tests/Persistence/InMemoryDbFixture.cs +++ b/tests/Marathon.Infrastructure.Tests/Persistence/InMemoryDbFixture.cs @@ -17,13 +17,19 @@ public sealed class InMemoryDbFixture : IDisposable public InMemoryDbFixture() { - // Keep a single connection open so the in-memory DB is not dropped between - // DbContext operations. Cache=Shared ensures the same DB is reused. - _keepAliveConnection = new SqliteConnection("Data Source=marathon_tests;Mode=Memory;Cache=Shared"); + // Each fixture instance gets its own in-memory database so xUnit's per-test + // isolation isn't violated when tests run in parallel within a class + // (the previous shared "marathon_tests" name caused state contamination). + var dbName = $"marathon_tests_{Guid.NewGuid():N}"; + var connectionString = $"Data Source={dbName};Mode=Memory;Cache=Shared"; + + // Keep a single connection open so the named in-memory DB lives until the + // fixture is disposed. + _keepAliveConnection = new SqliteConnection(connectionString); _keepAliveConnection.Open(); var options = new DbContextOptionsBuilder() - .UseSqlite("Data Source=marathon_tests;Mode=Memory;Cache=Shared") + .UseSqlite(connectionString) .Options; DbContext = new MarathonDbContext(options);