search

adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.84k stars 270 forks source link

Integrate yaml-test-suite testing #455

Open andrewimeson opened 2 years ago

andrewimeson commented 2 years ago

Should we add tests to run yamllint against the yaml-test-suite project? There's only one failure right now, brought up in #452, but adding it to the test suite could prevent regressions.

If yes, how should it be added?

  1. A submodule in the repository that is required to be present for tests to pass
  2. A submodule in the repository that if present will be used for testing
  3. A CI-only test handled in GitHub Actions (anti pattern?)

@adrienverge has expressed desire to keep the repository setup simple so that inexperienced contributors have an easy time making contributions, which makes me worry that a git submodule may be adding additional complexity.

adrienverge commented 2 years ago

Thanks @andrewimeson, that's an interesting idea!

Indeed, making yamllint status rely on an external repo is risky, as this repo may change independently. For this reason, I'm not a big fan of the first 3 options (although 1 and 2 make sense to me).

I see 2 others:

  1. Include generated YAML files from yaml-test-suite inside our tests/yaml-test-suite, and update them when needed. This is what we already do for official YAML spec examples, inside our tests/yaml-1.2-spec-examples.
  2. Regularly check yamllint against the yaml-test-suite files, and fix as needed. Like we did for https://github.com/adrienverge/yamllint/issues/452.

What do you think?

perlpunk commented 2 years ago

Option 2 I would recommend to use a submodule with a certain data release tag. I believe this use case is something submodules were invented for. Where do you see the risk with a submodule and a changing repo @adrienverge ? Copying the files into the yamllint repo will use a lot of space. The submodule could be optional, but a highly visible message how to enable it could be printed when tests are running without it.

adrienverge commented 2 years ago

Thanks for your input Tina!

Fixing the submodule to a certain release tag would be a solution. The only remaining risk I see would be building problems (in case make data-update suddenly stops working for any reason in the future), and the dependency to Docker (is there a way to avoid that?)

perlpunk commented 2 years ago

my point was to use a data release tag. example: https://github.com/yaml/yaml-test-suite/releases/tag/data-2022-01-17

the data exports have all files readily available. no need for make data-update. it's documented in the readme.

the master branch is for maintaining the test data, for which one file per test case can have advantages.

(by the way, a submodule is always fixed to a certain commit or tag)

adrienverge commented 2 years ago

In this case this could do :+1:

(by the way, a submodule is always fixed to a certain commit or tag)

Thanks, I didn't know that. Indeed this reduces the risk of things breaking overnight...