Tests for bin/hack --link

[gazayas]

This is a start for addressing bullet-train-co/bullet_train-core#510.

This doesn't cover bin/hack --link for custom paths yet, but it covers the base logic for paths pointing to local/bullet_train-core.

@jagthedrummer I'll open this one as a draft in case we want to add custom paths in this PR or edit this base test in a way that would be the most productive/beneficial for the script as a whole.

[gazayas]

I wanted to originally load the packages with YAML.load, but they're not being properly loaded on CI for some reason. I resorted to writing in literal strings like we do with bin/checkout-and-link-starter-repo.

[gazayas]

The HIDE_THINGS test is passing now, but that's mainly because I've added this conditional (CI links to path: "./core/bullet_train-core"):

if Dir.exist?("local/bullet_train-core")
  test "bin/hack --link links gems properly to local/bullet_train-core" do
    `bin/hack --link`
    ...
  end
end

All of the tests are working locally though, even when running with both CI=true and HIDE_THINGS=true.

[jagthedrummer]

I don't quite understand what this setup routine is supposed to do. If I insert a binding.pry at the end, after we're re-written Gemfile I'm not seeing that any actual change has been made. The line that says gem "bullet_train" prior to setup still says gem "bullet_train" after setup has run.

So I think that the main thing setup is doing currently is capturing the pre-test state of Gemfile so that we can restore it. If that's the case (and I'm not missing something) it might be simpler and easier to follow if we just explicitly made a copy of Gemfile and then restored it.

Running this test is also leaving my local repo in a dirty state, with uncommitted changes to Gemfile.lock, so we may need to restore that one as well.

Something like:

def setup
  `cp Gemfile Gemfile.pre_test`
  `cp Gemfile.lock Gemfile.lock.pre_test`
end

def teardown
  `mv Gemfile.pre_test Gemfile`
  `mv Gemfile.lock.pre_test Gemfile.lock`
end

[jagthedrummer]

Oh, I think maybe I understand why setup is the way it is.

In CI, due to the way bin/checkout-and-link-core-repo works the pre-test state of Gemfile would be saying:

gem "bullet_train", path: "core/bullet_train"

Instead of just:

gem "bullet_train"

And we'd want to keep pointing to core/bullet_train instead of local/bullet_train-core/bullet_train to ensure that we're testing against whichever branch of core we may have determined to use via our branch-matching process. Without that setup process we'd end up testing against the main branch, which is what we'd get by default when running bin/hack --link.

BUT since we don't already have anything in local/bullet_train-core we end up skipping this test entirely, which is probably not what we want in CI.

I think we need to remove that conditional and try to figure out a different way around whatever is causing things to fail.

[jagthedrummer]

Just had a thought about this. I kinda think that we can safely assume that bin/hack --link should always be working against a "clean" Gemfile. That is, one that starts out pointing to released gems, and not to random local paths.

If we're comfortable with that assumption then can we just copy test/support/temp_gemfile.rb over the top of Gemfile in setup and then restore the original state in teardown?

def setup
  `cp Gemfile Gemfile.pre_test`
  `cp Gemfile.lock Gemfile.lock.pre_test`
  `cp test/support/temp_gemfile.rb Gemfile`
  `bundle install`
end

def teardown
  `mv Gemfile.pre_test Gemfile`
  `mv Gemfile.lock.pre_test Gemfile.lock`
  `bundle install`
end

I think we'd also want to modify the bullet_train line in temp_gemfile.rb to say:

gem "bullet_train", path: "core/bullet_train"

[jagthedrummer]

This line is currently making it so that we don't run this test in CI since we don't have a local/bullet_train-core directory. In CI we currently clone the core repo into ./core so it might make sense to pass that as the link destination.

[jagthedrummer]

Scratch that. If we passed in ./core as the link destination we'd expect the end result to be zero changes to the Gemfile, so we wouldn't be actually testing any functionality. 🤔

[jagthedrummer]

Instead of maintaining this list it might suffice to just spot check two specific packages. For instance we could theoretically just test bullet_train-api as being representative of gems that are listed in the Gemfile and test bullet_train-fields as being representative of gems that are not listed in the Gemfile.

[gazayas]

Didn't get around to this one today, will try to address it soon.

[jagthedrummer]

No rush on this, imo, @gazayas. If there's other stuff that seems more urgent this can definitely wait.