[ovsp4rt] Public API functions are difficult to test

[ffoulkes]

Problem statement

The functions in ovsp4rt's public API are inherently difficult to unit test. This is because each function embeds explicit logic to establish a client connection to the P4Runtime server.

  // Start a new client session.
  auto status_or_session = ovsp4rt::OvsP4rtSession::Create(
      grpc_addr, GenerateClientCredentials(), absl::GetFlag(FLAGS_device_id),
      absl::GetFlag(FLAGS_role_name));
  if (!status_or_session.ok()) return;

  // Unwrap the session from the StatusOr object.
  std::unique_ptr<ovsp4rt::OvsP4rtSession> session =
      std::move(status_or_session).value();
  ::p4::config::v1::P4Info p4info;
  ::absl::Status status =
      ovsp4rt::GetForwardingPipelineConfig(session.get(), &p4info);
  if (!status.ok()) return;

In addition, all communication with the server is done via function calls that couple the code tightly to the client session.

This makes it extremely difficult to isolate the body of the function so it can be tested independently of the P4Runtime server.

Proposed solution

The classic solution to this kind of problem is to introduce an object that provides an abstract interface to the server, and then pass that object to a function containing the core logic, a technique known as dependency injection. We can then provide two (or more) implementations of the server interface: one that communicates with an actual P4Runtime server, and one (or more) that allow us to simulate the desired behavior of the P4Runtime server according to the needs of the individual test case.

To implement this, we introduce an ovsp4rt Client object (there's already a Session object) to handle the interface to the server, and we divide each public API function into two parts. One instantiates a Client object and passes it to the function that implements the core logic:

void ovsp4rt_config_fdb_entry(struct mac_learning_info learn_info,
                              bool insert_entry, const char* grpc_addr) {
  using namespace ovsp4rt;
  Client client;
  ConfigFdbEntry(client, learn_info, insert_entry, grpc_addr);
}

The other implements the core logic:

void ConfigFdbEntry(Client& client, struct mac_learning_info learn_info,
                    bool insert_entry, const char* grpc_addr) {
  absl::Status status;

  // Start a new client session.
  status = client.connect(grpc_addr);
  if (!status.ok()) return;

  // Fetch P4Info object from server.
  ::p4::config::v1::P4Info p4info;
  status = client.getPipelineConfig(&p4info);
  if (!status.ok()) return;
    .
    .
}

The test case can then exercise API by instantiating a test double of the Client object and passing it to the core function.

As an added benefit, we have reduced the amount of boilerplate that needs to be specified in each function, making the code both easier to read and easier to maintain.

Status

Draft PR #674 is work in progress toward implementing the proposed solution.