Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-68723

align SessionWorkflowTest with mocking of events, not guessing states

    • Type: Icon: Improvement Improvement
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 6.3.0-rc0
    • Affects Version/s: None
    • Component/s: None
    • Labels:
      None
    • Fully Compatible
    • Service Arch 2022-12-26, Service Arch 2022-08-22, Service Arch 2022-09-05, Service Arch 2022-09-19, Service Arch 2022-10-03, Service Arch 2022-10-17, Service Arch 2022-10-31, Service Arch 2022-11-14, Service Arch 2022-11-28, Service Arch 2022-12-12, Service Arch 2023-01-09, Service Arch 2023-01-23, Service Arch 2023-02-06
    • 60

      • SessionWorkflowTest has a confusing implementation because it's centered around "states", but its inputs are events (the virtual functions intercepted by mocks).
      • Rephrase the tests, particularly StepRunner-based suites, as a standard mock sequence. An expected event, and a scripted response, and then a sequence of such simple exchanges.
      • The use of a ProducerConsumerQueue is conceptually problematic. Replace this with a single optional<Event> guarded by a mutex and condvar. This constrains the possibilities and makes the test simpler to reason about.
      • Collapse the RequestKind and FailureCondition enums into one. They are the same thing: an encoding for a scripted response. Some are payloads (RequestKind) and others are Status or other error behaviors (FailureCondition). Code is much cleaner when these are one enum, and we don't have to carry two numbers around, one of this is only useful if the other is kNone. It's over-complicating a simple thing.
      • Continue the trend from SERVER-67426 of making callback-based mocks, but take even more functions out of the fixture and convert them into local logic in the mocks.
      • Make the mocks in the fixture's setUp, with private factory functions. Tests do not need to makeSession. They should get one for free from the fixture.
      • Compact notation for the StepRunner-based test specs.
      • Simplify the mock callback bodies to be one-liner calls to `_onMockEvent<Payload>(event)`. This replaces several sites at which the same exact code was replicated 4 times. Exploit the symmetry. They're all doing the same thing: notifying the main thread that an event has occurred, block waiting for that message to be received by the main test thread, and responded to, and then return that response through the mock function's return value.

      Lots of misc other refactors fall out of this.

            Assignee:
            billy.donahue@mongodb.com Billy Donahue
            Reporter:
            billy.donahue@mongodb.com Billy Donahue
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: