[DIF] tests

[silvestrst]

Just want to start the discussion on DIF testing. From previous conversations it seems that the consensus was to have several types of testing such as "mock" and DV.

I think DIF are ideally suited for "mock" testing, as all/most of the API calls will result in a single or multiple register read/write. A such it would should be easy to verify the API, by mocking out register write/read functions, and comparing the actual result with the expected. Expectations are that all the MMIO access should be done through the library introduced in #1187 (when it lands). This would make thing easier, as all of the DIFs will be using the API introduced in the mentioned PR.

This is just thinking out loud really, and invitation for comments.

[silvestrst]

@gkelly @asb @lenary @mcy and anyone else

[mcy]

I want to do something similar. I think the main thing we need is to add an injection point to mmio.h to replace it with a mock or fake version.

I suspect the right thing to compile separate "test" versions of each DIF, which we pass special -D flags to to enable an #ifndef in mmio.h that switches to the appropriate fake implementation.

If we instead wind up doing things in Rust, then the right thing to do instead would be for mmio.rs to define

trait MmioReg {
  fn read(&self) -> u32;
  fn write(&self) -> u32;
  fn read_mask(&self, ...) -> u32 { ... }
  ...
}

struct VolatileReg(u32);
impl MmioReg for VolatileReg { ... }

We could then make the DIF struct have a <Reg: MmioReg = VolatileReg> type parameter, which would make substitution trivial at test-time.

There's also the separate issue of interrupts, such as for rv_timer. I suspect the right way to do this is for the test harness to just manually "trigger" an interrupt by jumping to the interrupt handler.

[tjaychen]

It sounds like what you are talking about is a direct csr test. There are generally speaking, different types of csrs with different side effects, and also csrs that act like memory windows (backed by things like a FIFO).

Would you want to test the effect on those as well?

On Wed, Dec 18, 2019 at 2:11 AM Silvestrs Timofejevs < notifications@github.com> wrote:

@gkelly https://github.com/gkelly @asb https://github.com/asb @lenary https://github.com/lenary @mcy https://github.com/mcy and anyone else

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/1224?email_source=notifications&email_token=AAH2RSXY4EB4K5B3TBROZDDQZHZNXA5CNFSM4J4IP742YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHFTRGA#issuecomment-566966424, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSUHD6QLDCWLQL34TODQZHZNXANCNFSM4J4IP74Q .

[mcy]

For registers implemented as MMIO this isn't an issue, because we can just use the mechanism I described above to mess with mmio.h.

Interacting with CSRs, in the RISC-V sense, is a somewhat harder question. I expect we'll just have csr.h or similar that can be mocked out the same was as mmio.h.

[tjaychen]

sorry i should have clarified.. i meant to say 'mmio' registers have various types. Think for example read-write, read once, read clear, write clear etc. And indeed some of these registers map to FIFOs or other memories behind.

[alphan]

I've been thinking about this too. Echoing what @mcy said, we can use function pointers to replace actual implementations with mocks when building for tests.

@tjaychen, I think we can mock any interesting behavior to verify that code under test interacts with such registers properly.

[sjgitty]

@gkelly @asb @lenary @mcy and anyone else

@sriyerg

[alphan]

I've just sent a PR (#1254) with some unit tests. Looking forward to your comments!

[silvestrst]

@alphan thank you for introducing the first set of unit tests, we can now expand on the subject with more depth.

I personally envisioned that dif_device.{c, h} won't be polluted with any testing related functionality, and that any mock tests reside outside of the sw/../lib/dif directory.

Can't we have mock tests built as separate libraries, and at the build time, instead of using real mmio.c link against one that introduces stubs.

Also I guess unit tests should be only built and ran as a part of a debug build?

[alphan]

I personally envisioned that dif_device.{c, h} won't be polluted with any testing related functionality, and that any mock tests reside outside of the sw/../lib/dif directory.

Can't we have mock tests built as separate libraries, and at the build time, instead of using real mmio.c link against one that introduces stubs.

Yes, that's possible. But, by introducing function pointers, we can write more focused test points that test only the new behavior that a function implements. E.g., imagine that we have functions util_a() and util_b() which are static utility functions implemented in dif_device.c and called by almost every other function implemented in that file. First, we write tests to verify them. Then, for testing the public functions dif_device_func_1() ... dif_device_func_n(), we can replace a and b with mocks, just verify that they are called with the right arguments, and dedicate each test to verification of the new functionality that the corresponding function provides. The main benefit of doing so is increased maintainability -- if we change the behaviors of util_a() or util_b() we only need to touch their tests. The main drawback is the pollution in dif_device.c (because we don't want these function pointers in normal builds). If we just mock mmio.c, we would have to effectively inline tests for util_a() and util_b() in tests for dif_device_func_1() ... dif_device_func_n(). For some DIFs, this may not matter at all but I thought it would be significant for GPIO. What do you think?

P.S. I forgot to mention that we need this preprocessor trick only for the static functions that we want to mock, so the impact on dif_device.c will depend on the actual implementation. If there are no static functions, then we can probably just replace dependencies with mocks during build.

[mcy]

Also I guess unit tests should be only built and ran as a part of a debug build?

Tests should run for all builds. If a test passes at -O0 but fails at -Oz, we've got a big problem.

[silvestrst]

Attempt to summarise the previous discussion on DIF mock testing. Consensus has been reached that only public API should be tested. The DIF (source and header) should be used as is by a testing framework.

There are only several low level libraries that DIFs should depend on (mmio, ...), so for testing purposes only public API in these libraries should be able to be mocked. See #1269 for an example. Unfortunately it seems that it's not possible to entirely avoid testing related preprocessor conditionals in these libraries, but these are fairly minimalistic.

I think that we are leaning towards have all the mock testing done in CI on x86. I personally think that this is a right approach, as I don't see any benefits of running these on the target.

Please let me know if you think something is not entirely right in this summary, and if I have missed something.

[silvestrst]

Had a look at following test frameworks: googletest(C++), greatests (c89), theft(c), check(c)

Googletest major positives:

• Well documented
• Is used by large projects (llvm, OpenCV, ...)
• xUnit format
• Can be built as a library
• As far as I understand don't need to create runners if testing all tests (might be wrong).
• Provides xml output, but also allows pretty straight forward way of changing the output format. There is a separate mini project that implements a plugin for googletest to provide output in TAP format
• Used by google, PRs open with googletest unittests (GPIO, SPI)

greatest major positives:

• Only uses features in C89
• Single header less than 1000LOC
• Good documentation
• Script included to format the output to TAP negatives:
• no XML output

Check major positives:

• Implemented in C
• Good documentation, however slightly more awkward than googletest
• XML, TAP output negatives:
• Not sure if can be compiled as a library out-of-the-box

In general all of these options seem good. All of the above options are fairly easy to use, and I don't think there is too much difference. I think googletest probably is the way to go, as it is already being used in some of the PRs and there is no concrete arguments to not go with it. I think there is probably not much benefit of running mock tests on the target, and when rust comes potentially we would need to reconsider the test framework anyway.

[lenary]

Given the current proposed unit testing mechanism (running mock tests on hosts, not RISC-V devices), I am happy for us to proceed with googletest/googlemock.

[lenary]

This was mentioned in the software meeting on 21 Jan 2020. We are all happy to proceed with googletest/googlemock.

[lenary]

For Mock testing, googletest has now landed (#1345) and so has the mocked mmio_region_t (#1298).

There is further coordination going to happen between DV and software tests, which we should understand better soon.

[silvestrst]

This issue is stale, and forgotten.