TheThingsNetwork / lorawan-stack

The Things Stack, an Open Source LoRaWAN Network Server
https://www.thethingsindustries.com/stack/
Apache License 2.0
960 stars 302 forks source link

Flow tests in components #176

Open rvolosatovs opened 5 years ago

rvolosatovs commented 5 years ago

Summary:

Apart from isolated unit tests should contain flow tests, where various common/expected flows are tested. For example in NS context that could be an example:

  1. Create a device via Set(possibly multiple devices)
  2. Call HandleJoin
  3. Call HandleUplink
  4. Call ApplicationDownlinkPush
  5. Call HandleUplink

Some rules:

Note, that this approach to testing does not replace isolated white-box unit testing and both are equally important.

Why do we need this?

To ensure our components do what we(and our users) expected them to

What is already there? What do you see now?

https://github.com/TheThingsNetwork/lorawan-stack/blob/f1377b316a85ae3709a84db5b5f9a46faafba3a0/pkg/networkserver/registry_test.go#L34-L235 is an example of a "flow test", but on a lower level and it only tests one flow. We need something similar, but on component level and testing several distinct flows.

What is missing? What do you want to see?

Flow tests

How do you propose to implement this?

Write/adapt the tests described

rvolosatovs commented 5 years ago

@johanstokking @htdvisser please look through the proposal and give your opinion

htdvisser commented 5 years ago

Apart from the fact that your summary contains the content I'd expect in "What is missing" and "How do you propose to implement this", this sounds fine to me. Of course we would have to skip tests of infeasible implementations (just like we don't test S3 buckets in the blob package if there is no configuration for it).

johanstokking commented 5 years ago

I find this important enough to scope this to NS and assign the core team. This requires also in depth understanding of LoRaWAN, NS, the way NS is tested now, etc, to call for community help.

Keeping it in Backlog now, but we need to have it, if only for automated regression testing.

Suggesting to put AS in here as well, but let's keep this flow testing and not integration testing.