clash-lang / clash-lang/clash-protocols

Revamp testing infrastructure

Open
#144 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Haskell
Stars
26
Forks
12
PR merge metrics
No merged PRs in 30d

Description

Back when I introduced `clash-protocols` and `Df`, I realized that many of the circuits I was writing had a pure equivalent. For example, `fifo :: Circuit (Df dom a) (Df dom a)` ~ `id`. I figured I'd generalize this construct into `propWithModel` and friends. For the places where it has worked, it has been pretty great, but it has a bunch of problems:

* It assumes quite a bit about the structure of your circuit.
* It has complicated type class machinery (`Test` / `Simulate` / etc.)
* It tries to generalize all possible protocols into a single concept (e.g., it assumes a single backpressure channel, [Why there is no `Drivable` instance for AXI](https://github.com/clash-lang/clash-protocols/blob/8b6a7695161c2bada9d1373c6fcaf0da887c787a/README.md#why-there-is-no-drivable-instance-for-axi))
* It doesn't work all that well with Hedgehog, [relying on a fork of it](https://github.com/clash-lang/clash-protocols/blob/8b6a7695161c2bada9d1373c6fcaf0da887c787a/README.md#debugging).
* It evaluates channels one-by-one, leading to massive memory use for longer running simulations.

So all in all, we can do better. For round two, I think _running_ the circuit should be separate from _building_ the test bench. For example, a FIFO test harness would look like:

```haskell
dut ::
DriverOpts ->
ExpectOpts ->
[a] ->
Circuit () (Vec n DriverSamples)
dut driverOpts expectOpts dat = circuit $ do
fifoIn <- driver clk rst driverOpts dat
fifoOut <- fifo clk rst -< fifoIn
results <- expect "fifo" clk rst expectOpts dat -< fifoOut
idC -< [results]
where
clk = clockGen @XilinxSystem
rst = noReset
```

The Hedgehog test would look like:

```haskell
prop_dut :: Property
prop_dut = property $ do
driverOpts <- forAll genDriverOpts
expectOpts <- forAll genExpectOpts
dat <- forAll genDat
testResult <- liftIO $
runTest
(testOptions{generateVcd=True})
(dut driverOpts expectOpts dat)
```

where

```haskell
runTest ::
(1 <= n, KnownNat n) =>
TestOptions ->
Circuit () (Vec n DriverSamples) ->
IO TestResult

-- | A 'DriverSamples' is a collection of samples and their timestamps, as well as the
-- status of the driver. It is used to record traces of signals in a simulation.
--
-- TODO: Support different active edges.
data DriverSamples = DriverSamples
{ name :: Text
-- ^ Name of the driver
, traces :: [Samples]
-- ^ (Optionally) traces associated with this driver
, period :: Maybe Femtoseconds
-- ^ Period of the clock. Set if dealing with classical clocks (i.e., non-dynamic).
, statuses :: Infinite (Femtoseconds, Status)
-- ^ Statuses with their timestamps
}

data Sample = Sample
{ value :: !Natural
, mask :: !Natural
}
deriving (Eq, Show)

data Samples = Samples
{ name :: Text
-- ^ Name of the signal
, bitSize :: Natural
-- ^ Size of the signal element in bits
, period :: Maybe Femtoseconds
-- ^ Period of the clock. Set if dealing with classical clocks (i.e., non-dynamic).
, samples :: Infinite (Femtoseconds, Sample)
-- ^ Samples with their timestamps
}

data Status
= -- | Do not shut down simulation.
PreventStop
| -- | Error condition. Driver is free to go to any other state after
-- asserting this.
Error Text
| -- | Shutting down simulation is OK. Driver may still be monitoring
-- for errors.
StopOk
deriving (Eq, Show)
```

Though there is now more boilerplate to construct a test, it doesn't suffer from any of the issues `propWithModel` and friends suffer from:

* It does not assume anything about the design under test
* It does not rely on any type classes to be implemented
* It does not try to generalize protocols, instead relying on protocol-specific drivers and expect circuits.
* It moves all "fuzzing" to a user-written test, making it work well with Hedgehog again.
* It can step through the simulation in constant memory. All `Streams`s contain timestamps of their samples, allowing `runTest` to evaluate all samples in time-based order.

As a bonus, `runTest` can also access traces, paving the way for generating waveforms of failed tests. As a cherry-on-top, circuits can export traces too making test failures much easier to debug.

# TODO
There is already a bunch of code in [martijn/revamp-test-infrastructure](https://github.com/clash-lang/clash-protocols/tree/martijn/revamp-test-infrastructure).

- [ ] Implement drivers and expecters for `Df`
- [ ] Replace `propWithModel` tests in testsuite of `clash-protocols` with structure mentioned here.

# Future
- Expand support to all protocols in `clash-protocols`
- Implement "monitors": circuits that are only there for waveform purposes.
- Extend with scoreboards: tracking functional coverage across multiple hedgehog tests
- Optimize evaluation strategy: we could group signals per clock (domain), dropping the need to calculate periods for every sample/trace.
- Explore whether `Samples` should be a tree. That way we could collect a bunch of traces in the design and have waveform viewers reflect the structure of the design.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by inspecting the martijn/revamp-test-infrastructure branch and the existing propWithModel tests in the clash-protocols testsuite. Focus first on the TODO items for Df drivers and expecters, then replacing the existing tests with the proposed structure. Done means those tests use the new infrastructure and the Df test path works.

Written by the indexing model from the issue text.

Assessment

Tech stack
haskell
Domain
testing-qa
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.