Re: [LTP] [PATCH] thermal: add new test group

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi!

There are a few issues with this code. We need to clean it up
before proceeding with the proper review. In LTP we have a strict policy
when it comes to writing new tests and that can be read in the following
links:

https://linux-test-project.readthedocs.io/en/latest/developers/ground_rules.html
https://linux-test-project.readthedocs.io/en/latest/developers/writing_tests.html
https://linux-test-project.readthedocs.io/en/latest/developers/test_case_tutorial.html

Documentation is away from being perfect, but it needs to be read before
proceeding, since some technical aspects are covered only in there and
inside some of our reviews, especially the ones from maintainers with
the highest expertise (@Cyril, @Petr and @Li in particular).

A few points below:

* run `make check` locally inside the test folder and verify errors
  inside the test code

* run github CI by pushing your commits inside a forked LTP project.
  The patch is not passing in any of our tested platforms (I'm sorry if
  CI report is not working in patchwork yet, but that has been fixed
  today)

* please verify that all you need has not been implemented inside the
  LTP library already. For instance, we already have `tst_ncpus()` and
  _SC_NPROCESSORS_ONLN is not needed

* please avoid as much as possible unsafe memory operations over strings.
  we dont really want to use strcpy(), strcat(), etc. In most of the
  cases, snprint() and asprintf() are the right choice

* why using regexp for something as easy as finding a substring inside a
  string, without any particular substring rule over data?

In general, we need to create a code that is working as good as possible,
since debugging test failures can be really complex inside kernel. Test
has to provide very little friction with the kernel in order to reduce
test maintenance.

Let's start from this, we will continue the review when these points are
first achieved.

Kind regards,

-- 
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp