urbdyn / rules_fish

Bazel toolchain rules for Fish shell.
Apache License 2.0
1 stars 0 forks source link

Bootstrap Fish shell's built-in tests #3

Closed Matthew-Benson closed 1 week ago

Matthew-Benson commented 3 weeks ago

We can have much higher confidence in this library and support for various fish versions if we can run Fish's provided shell tests for various platforms with various versions in CI, but to do that, we'll likely need to enhance the cmake rule to capture the fish_test_helper binary from the build and make an internal, private rule to use that to run Fish's tests.

Ideally, we run the tests from https://github.com/fish-shell/fish-shell/tree/master/tests - especially tests in the checks directory.

Matthew-Benson commented 2 weeks ago

We can follow this - https://github.com/fish-shell/fish-shell/blob/master/CONTRIBUTING.rst#testing and port their cmake testing script to bazel build targets, but there are many layers of indirection in the source project to untangle.

Matthew-Benson commented 2 weeks ago

We need to write a shell script to replicate this logic: https://github.com/fish-shell/fish-shell/blob/ef614ba2d95377748834caca7307ef6c799ba1c3/cmake/Tests.cmake#L105-L117

foreach check
  sh ./tests/test_driver.sh tests/test.fish ${CHECK}

There's one constraint that we will have to meet for this to work: these tests expect all the directories and paths to be laid out a particular way relative to the test root directory. We will need to copy or symlink out all the files we need to build this structure as it's not exactly what you get with our build of fish with bazel.

Attempted to roll our own solution in #29, but going to need to rework and use their test harness.

Matthew-Benson commented 1 week ago

Ah, we can instead use the cmake tests as-is, just would prefer to find a way to make that a "bazel test".

Matthew-Benson commented 1 week ago

There's a working solution in #29, but we can improve this by wrapping our "not-a-test" test with a custom rule. See docs: https://bazel.build/rules/testing#using-custom-rule

Matthew-Benson commented 1 week ago

Resolved by #29