Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_ecosystem should record the last working shards #116

Open
beta-ziliani opened this issue Aug 31, 2021 · 3 comments
Open

test_ecosystem should record the last working shards #116

beta-ziliani opened this issue Aug 31, 2021 · 3 comments
Assignees

Comments

@beta-ziliani
Copy link
Member

Right now it just runs on the latest release of shards, but that doesn't guarantee that if something fails it's our fault.

@straight-shoota
Copy link
Member

straight-shoota commented Sep 1, 2021

Currently, test_ecosystem clones and checks out all shard repositories from a central script: https://github.com/crystal-lang/test-ecosystem/blob/94560278641b6295441c7bce6d4d715ff05e808b/scripts/10-clone-repos.sh

This means when adding a new test, you need to add a bats test and also to clone the repository in this central script. It would be better if adding a test happens at a single location. I think we can improve on that in the process of reworking the way code is pulled in.

Keeping track of installed versions can be easily accomplished by using shards to download the test repositories.
We can't use a single shard.yml for all of them, because there could be dependency conflicts. Every tested shard needs their own shard.yml which typically just contains the tested shard itself.

This would be an example file for crinja:

# shards/straight-shoota/crinja/shard.yml
name: test-crinja
version: 0.0.1

dependencies:
  crinja:
    github: straight-shoota/crinja

The workflow for the test would look like this:

@test crinja {
  cd shards/straight-shoota/crinja
  shards install
  cd lib/crinja
  make test
}

shards install installs crinja with all of its dependencies. By cd lib/crinja the script enters the installation directory of crinja and there we can run any commands as with the current setup in $REPOS_DIR/straight-shoota/crinja.

shards install creates a shards/straight-shoota/crinja/shard.lock which can be checked in to git for ensuring consistency, not only of the tested shard itself but also all of its dependencies.

The file system setup becomes more complex because we need a separate directory for every tested shard.
Technically, this still requires editing two different files (bats and shard.yml) when adding new a shards. But I think the creation of shard.yml can actually be automated. The bats test would call a helper script that creates the shard.yml if it does not yet exist, runs shards install and points the working directory to the installation path.

Usage from bats test:

@test crinja {
  enter_shard straight-shoota/crinja
  make test
}

Helper script:

# enter_shard

repo=$1
name=${2:-${repo%/*}}
repo_dir = "$REPOS_DIR/$repo"
shard_yml = "$repo_dir/shard.yml"

if [ ! -f "$shard_yml" ]; then
  cat <<-YAML > "$shard_yml"
    name: test-$name
    version: 0.0.1

    dependencies:
      $name:
        github: $repo
    YAML
end

cd "$repo_dir"

shards install

cd "$repo_dir/lib/$name"

@bcardiff
Copy link
Member

bcardiff commented Sep 1, 2021

I like the attempt to have a single location for declaring repo and test for a shard.

I find it odd the proposal to user a shard.yml to do that. I would expect to mimic the action of the user to clone and run tests.

The webframeworks bats has some dependencies between each other. Building cli before smoke testing an empty app. Let's be sure we don't prevent that from keep happening.

I didn't want to lock the subject shard. Maybe this is more useful in a 0.x era were it was better to keep testing master rather than latest release. So this can change, but the current clone repos allow branch/tag/ref/fork to be tweaked in a case by case basis.

@straight-shoota
Copy link
Member

straight-shoota commented Sep 1, 2021

I find it odd the proposal to user a shard.yml to do that

The main motivation for this is that it allows to easily lock versions by checking in shard.lock. And to update them you just have to run shards update for every shard folder.
You can also just run shards update to ignore the locked version.

I'm not married to using shards for this, but it seems a decent option.

And it pretty accurately represents the way that a library shard is installed (as a dependency).

Building cli before smoke testing an empty app. Let's be sure we don't prevent that from keep happening.

Yes, definitely. Assuming everything is set up correctly, enter_shard should be idempotent. You should be able to run it sequentially times in different tests without changing outcome.

The reason for locking is to increase resilience against bugs introduced in new shard releases, and thus improcing confidence that a breaking test is actually caused by bug in the compiler or stdlib, not the shard.

This could allow us to incorporate test_ecosystem as an automated validation for release workflows in CI (#115).

Pinning specific refs should continue to be possible, that would just be a restriction in shard.yml (via the helper). Same goes for overrides.

I think it should even be relatively easy to test different versions of a shard. For that we'd just need to disambiguate different versions for the working directory. Either include the version restriction or base the path on something else, like the name of the bats test.

@straight-shoota straight-shoota self-assigned this Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants