Skip to content

Functional tests should be split to be more granular #670

@stof

Description

@stof

Our functional tests are based on input files in https://github.com/scssphp/scssphp/tree/master/tests/inputs with a corresponding output CSS file in the outputs folder. Note that it does not support creating nested folders (the subfolders there are for partials that are imported by some tests).

Today, those tests are often not granular at all, making it hard to maintain the tests:

  • it is not clear what the test is testing
  • failures are all-or-nothing even when the test covers 20 different cases
  • often, the generated output does not even make it easy to identify which part of the file failed (in particular for builtins.scss which creates selectors with many times the same property so we don't know which one failed).
  • it makes it harder to compare the output to the reference implementation, for 2 reasons:
    • again, the all-or-nothing issue
    • for some cases, the failure is only related to formatting differences related to how different blocks are separated by empty lines or no. This is mostly solved for the compare-scss.sh file because it attempts at ignoring those empty lines. But this will affect us again when migrating to the new rendering in 2.0 which will change the formatting, leading to many file changes.

In addition to that, many things are already covered by equivalent (granular) tests in the official sass-spec testsuite.

Note that we also have some tests that are already properly granular (the more recent ones, as I stopped adding more things in the existing tests).

Work to do

extending_compound_selector.scss, color_operators.scss and comment_incomplete_interpolation.scss should not be reworked. They are covering deprecated behaviors of scssphp that are triggering errors in dart-sass (and so will also trigger errors in scssphp 2.0) so it is not worth spending time on them.

For each test that is not yet granular:

  1. identify what it attempts to test (for some of them, it might related to identifying the issue for which they act as a regression test). this will generally produce a list of things
  2. for each of those tested things:
    1. Check whether there is a test in the official sass testsuite.
    2. If yes, check whether we run that test or no in our testsuite (run phpunit -v tests/SassSpecTest.php --filter <name_of_the_test> and check whether the test is skipped or no. A quick check can also be done by checking in tests/specs/sass-spec-exclude.txt whether the test is excluded, but this exclusion list does not cover all skipped tests). If we run it, our duplicate functional test should be deleted as duplicate. If we skip it (often because of a formatting difference in the output), add a functional test using the input from the official testsuite, with a comment mentioning the name of the official test. See an example here
    3. If no, extract that in a dedicated scoped test, with a name describing what the test is covering. Please keep the file name short enough to satisfy filesystem requirements to avoid messing with contributors. If needed, the test file can start with a Sass silent comment (i.e. using //) providing a longer explanation
    4. If the new test is a regression test for a particular bug, it is good to provide a link to the issue in a silent comment at the beginning of the input
    5. Remove the old test code for that thing

A good check to do is to run the tests from tests/InputTest.php with code coverage enabled before and after the extraction of the granular tests, to ensure we don't regress on code coverage when splitting the test (when deduplicating based on sass-spec, the coverage check is harder to do include those specific tests too)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions