Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Generated?) Unit tests #67

Closed
SiebrenW opened this issue Feb 6, 2024 · 4 comments
Closed

(Generated?) Unit tests #67

SiebrenW opened this issue Feb 6, 2024 · 4 comments

Comments

@SiebrenW
Copy link
Contributor

SiebrenW commented Feb 6, 2024

Here's a request to see if we can maybe add automated unit tests (not necessarily added to gh pipeline yet) to test the generated code with some test framework (I'm not sure what EVerest is used to. I use a lot of GTest, but its build times leave something to be desired).

The proposal is to:

  • Have a script that builds the library cbv2g
  • Have a bunch of unit tests (in C, C++ or python) to test the generated code. ie:
    • Encode specific messages and compare to a known good output
    • Decode a known stream and check the fields
    • Encode then decode and compare the input and output
  • The last type may even be generated by the generator (a procedure that sets a default value in each field and a test that generates the test.

I may want to work on this if you guys think this is a good idea (you'd be able to enable/disable the generation of these tests of course, not running existing ones would also make that optional)
Also do tell what test framework you'd like me to use. I know GTest, but I love to learn something new too.

@SebaLukas
Copy link
Collaborator

Since I also had the idea, I think it's generally a good idea.

But I don't know if the cbexigen repo is the right repo for it. Because the c code has to be generated every time.

My plan was to open another repo in EVerest with the generated cbexigen code. And to create a library "libexi" from it, which can then use e.g. the libiso15118 lib or the EvseV2G module. That's where I would see the unit tests.

In Everest we have not decided on a single unit testing framework. Some use Gtest, I use catch2 for the unit tests in libiso15118. So use the test framework that makes the most sense for you.

@SiebrenW
Copy link
Contributor Author

SiebrenW commented Feb 6, 2024

I don't really see the issue with re-generating the code every time. We can consider it another build step.
Keeping the code "as is" to me is like keeping a binary in the repo, the C code is supposed to be the product of the build step, in my opinion, like a binary executable is.
The code in the other repo would go out of date with every commit, adding a step of updating that repo every time this repo is updated. I think that's not ideal.

What these tests should be testing is:

  • Is the code still generating without errors?
  • Can this code build?
  • Did the API change? (may be fine, but we should know and a build error will tell us that)
  • Does the generated code do what we expect?
  • This test script can be used as reference to how to use this repo (config, generate, use)

If we put the generated code in another repo we can't test the first test automatically, it all involves manual effort.

So I do see a lot of advantages to at least link the tests directly to this repo. This can be as a submodule of the test repo and still have those tests generate the schemas, but again, that's extra effort to update the submodule every time.

Or am I missing something? Or outright wrong 😐

@SebaLukas
Copy link
Collaborator

I think I understand your point now. My focus is more on a heavy testing of the generated exi c code. In other words, a kind of fuzzing testing. That I can be sure that the exi parser can en- or decode something strange.

If it's about testing this cbexigen generator and its output, then that should definitely be here in this repo.

So I'm generally in favor of having those tests you mentioned above here.
From my side, you are welcome to write such tests and open pull request(s) for them.

@SebaLukas
Copy link
Collaborator

There were also no objections from the EVerest Car<->Charger Communication WG. I'm looking forward to your PR(s) 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants