libyaml icon indicating copy to clipboard operation
libyaml copied to clipboard

A new pinning system for the yaml-test-suite

Open ingydotnet opened this issue 4 years ago • 6 comments

The new system uses 2 branches instead of 3. They are master and run-test-suite.

Pinning is no longer in tsv file.

There are pin files for each supported master commit in the env dir. A pin file names the yaml-test-suite repo and data branch commit to use. It also lets you blacklist/whitelist which tests to use.

The pin files can be symlinks. They can also include (source) and override other files.

To specify pinning info for a PR you put it in your commit message. See ReadMe.md for more details.

ingydotnet avatar May 31 '20 17:05 ingydotnet

This branch should be skipped by travis and semaphore. For some reason it is skipped by gh-actions.

ingydotnet avatar May 31 '20 18:05 ingydotnet

To test this run-test-suite-2 branch, I created a master-2 branch.

It passes tests on Actions: https://github.com/yaml/libyaml/runs/725313313

ingydotnet avatar May 31 '20 18:05 ingydotnet

Use case:

I want to fix something. Let's say I expect no changes related to test suite. I checkout a feature branch, make my fix, run make test-all and it works. I commit my changes, run make test-all again and it fails because it can't find the commit.

Workaround: make test-suite env=pin-current...

For me, that is "just" annoying. Someone else probably has no idea why it doesn't work. Even if they find out how to add the pin in the command line, if they push their changes to their repo, CI will fail. So they have to find out how to add a new pin file to their forked run-test-suite branch. Whenever they edit their fix and make a new commit, the pin file has to be changed. That's a high burden for contributors. It also means they have to keep not only their forked master in sync with the original repo, but also the run-test-suite branch. People will ask why tests are failing, and I will have to be the one who answers.

perlpunk avatar Jun 01 '20 10:06 perlpunk

Use case 2:

Contributor finds out how to add the new pin file to their forked run-test-suitte branch. CI passes on their fork. Demo: https://github.com/perlpunk/libyaml/actions/runs/121361971

They make a pull request and it fails.

See #190 https://github.com/yaml/libyaml/runs/727054200?check_suite_focus=true#step:9:57

perlpunk avatar Jun 01 '20 11:06 perlpunk

Use case 3 (followup-to 2):

One of us can add the new pin file to the origin/run-test-suite-branch and restart the various CI jobs, so that they pass. Now the PR is merged in the release branch. If the "squash" feature is used, it will have the PR number appended to the commit message, making it a new commit id. CI will fail.

Even if the merge can be done without giving it a new commit id: If the release branch has to be changed, all commit ids after the change will be gone, and all pin files need to be updated.

perlpunk avatar Jun 01 '20 11:06 perlpunk

@perlpunk thanks for the use cases.

I think that 551356d addresses your use cases. The lookup now defaults to the latest env.

The contents of tests/run-test-suite/env/ are:

default -> env-2020-05-24-test-suite-9fe5172ca31
env-2020-05-24-test-suite-9fe5172ca31
pin-0.2.4-72e2f75277ff88ad1fb8a81cf422df06bf371578 -> env-2020-05-24-test-suite-9fe5172ca31
pin-master-9deee01508bb3b8ad25822f53d91d9f54eae9a0b -> env-2020-05-24-test-suite-9fe5172ca31

The file env-2020-05-24-test-suite-9fe5172ca31 contains pinning and blacklist info for that test-suite commit (the latest).

The default file is a symlink to that.

The pin-* files can be symlinks or they can source and override.

The system will show a big warning when it defaults.

To try this out use

  git checkout ingy.test-suite-support
  export LIBYAML_TEST_SUITE_RUN_BRANCH=run-test-suite-2
  ln -fs .makefile GNUmakefile
  make distclean
  make test-suite

I'm looking forward to your feedback...

ingydotnet avatar Jun 01 '20 18:06 ingydotnet