Skip to content

Unclean Tests #1128

@CDanU

Description

@CDanU

I think that tests should be as small as possible to precisely keep exactly one behavior in check. Currently the Uncrustify tests are far from that.

There are several multi kilobyte sized config files in the tests that where seemingly just dumped in there. This makes it really hard to figure out what is actually being tested. At this point it can only be guessed according to the filenames. It is hard go estimate how much test coverage we actually have.

I have seen:

  • config and source files that have no comments / documentation
  • config and source files that are not used
  • duplicated config files

and most importantly

  • several tests that do not actually test what they should

I therefore started to reduce the mess a bit.

  • Step 1: Shorten config files

I started to use the emscripten interface to strip off default values. It had the problem that it could not handle includes, transformed config variables into actual values and removed comments . I had to do this process manually in order to not loose this info. Seeing that this is not removing unneeded options I wrote a script that removes those from the config one by one while testing if Uncrustify is still able to generate the expected output.
Currently is is not always working as expected, as it sometimes removes too much / not enough options. Sometimes it needs to run multiple times. I am not going to include it into a PR, but I will store it here:
option_reducer.py

While running that script I found some (possibly) new unstable behavior and in two tests endless looping.

  • Step 2: Restore negative case options

Most useless options are removed from the config after step 1, that however unfortunately also includes negative case options, i.e. options that are purposely placed in the config to generate wrong output when they are activated. Those options need to be restored and documented.

  • Step 3: Reduce test sources

Unneeded options still persist at this step as they are on the one hand needed to generate the expected output (mostly indenting) but on the other hand are not actually used to test an underlying issue. This should not be the case.

  • Step 4: Fix faulty tests

Here is the branch that I am working on: clean-test-configs


In the meantime I would like it if someone could look up the following config files:

sp_for_semi.cfg                    unused since    : git-svn-id: https://uncrustify.svn.sourceforge.net/svnroot/uncrustify/trunk@850 74908cc9-670d-0410-99d4-ca74
stdcall.cfg                        test removed in : dc65b7c9ebfbc6964b9296f37dbf7ebbe227c1ff
(TODO) verbatim_strings.rerun.cfg  unused since    : 767b5343bde45be1bc020ed641dad434c244bc2b
blc_1.cfg                          unused since    : git-svn-id: https://uncrustify.svn.sourceforge.net/svnroot/uncrustify/trunk@293 74908cc9-670d-0410-99d4-ca74

I don't get what actually is the bug in here: https://sourceforge.net/p/uncrustify/bugs/557/
The config can be reduced down to

sp_before_dc                    = remove
sp_after_dc                     = remove

The problem in the bug description however is related to align_func_params = true.

Metadata

Metadata

Assignees

No one assigned

    Labels

    EngineeringAnything to do with building/releasing/deploying uncrustify; with CI; or with the test framework.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions