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

How to run with docker #28

Closed
vinibispo opened this issue Sep 15, 2022 · 21 comments
Closed

How to run with docker #28

vinibispo opened this issue Sep 15, 2022 · 21 comments
Labels
enhancement New feature or request

Comments

@vinibispo
Copy link

Hey, incredible work on this repository, and I think it would be better if we could run with docker, too, because the vast majority of companies use docker nowadays.
I thought like we could run this, setting custom_command as docker
Like:

require("neotest-rspec")(
{ custom_command = "docker exec -it mytestcontainer rspec" }
)
olimorris added a commit that referenced this issue Sep 16, 2022
@olimorris
Copy link
Owner

olimorris commented Sep 16, 2022

@vinibispo - I've made it possible to have a custom rspec command in the latest commit. Please let me know if this works

@pBorak
Copy link

pBorak commented Sep 20, 2022

@vinibispo Have you been able to use neotest-rspec alongside docker with this command?
I am having issues with paths - neotest uses absolute ones.
nvim-neotest/neotest#89

@vinibispo
Copy link
Author

I'll test it, sorry for the delay @olimorris

@olimorris
Copy link
Owner

Yeah my hunch is that absolute paths are going to be the killer here...

@evertonlopesc
Copy link

@olimorris I did the following test:

local neo = require('neotest')
local rspec = require('neotest-rspec')

neo.setup({
  adapters = {
    rspec({
      rspec_cmd = function()
        return vim.tbl_flatten({
          "docker exec -it <container> bin/rspec ",
        })
      end
    }),
  }
})

I have this return:

image

I also did the following test:

neo.setup({
  adapters = {
    rspec({
      rspec_cmd = function()
        return vim.tbl_flatten({
          "docker",
          "exec -it",
          "container",
          "bin/rspec"
        })
      end
    }),
  }
})

I have this return:

image

Do you have any other test ideas?

@pBorak
Copy link

pBorak commented Sep 21, 2022

@evertonlopesc
I have forked this repo in the past, tweaked the command to use docker, and had the same results as you. This is due to the absolute paths :/ the issue I mentioned here.

@olimorris olimorris added the enhancement New feature or request label Sep 21, 2022
@olimorris
Copy link
Owner

Let's keep this open for now and monitor this issue as @pBorak has kindly highlighted

@evertonlopesc
Copy link

@pBorak I read this comment and also your issues on neotest.

@olimorris ok, I got it

Thanks for your attention!

@olimorris olimorris closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2022
@mmirus
Copy link
Contributor

mmirus commented Dec 1, 2022

Hey, all! Short term, my solution is to use a shell script as my rspec executable, and mutate the paths as needed in that script.

Example below. You'll obviously need to substitute your own particulars.

#!/bin/bash

# Strip "/home/mmirus/git/repo/path/" from test paths sent to container
args="$(echo "$@" | sed 's/\/home\/mmirus\/git\/repo\/path\///g')"
docker compose exec web bundle exec rspec $args

Then you just do this:

require("neotest").setup({
  adapters = {
    require("neotest-rspec")({
      rspec_cmd = "my-script.sh",
    }),
  },
})

This is basically the formula I've used in the past with other plugins and editors that had trouble with running tests or tools in a container. Tell them to run a shell script as the executable, do any arg transformations needed in the script, then call the container with the final result.

It seems to work here, but it's barely tested.

Thanks for this adapter, Oli!

@pBorak
Copy link

pBorak commented Dec 2, 2022

@mmirus
Hey, thank you for your comment, really smart move with this script ❤️
I tested your workaround and it seems to work for running specs in general. Unfortunately, I've noticed that neotest test results do not work correctly - all tests are reported as failed. Is that functionality working for you?

@mmirus
Copy link
Contributor

mmirus commented Dec 2, 2022

I did see that too. I haven't had time to look into it yet. The most obvious explanation is that something about the output from docker (or the script) is not as expected. Perhaps we can capture "normal" output and output from the script and compare.

@olimorris
Copy link
Owner

olimorris commented Dec 2, 2022

@mmirus could be worth you posting this on the Neotest issue regarding Docker if you haven't done already. Your approacapproadh is brilliant and perhaps @rcarriga mayshave some thoughts regarding the output.

@mmirus
Copy link
Contributor

mmirus commented Dec 2, 2022

Good suggestion! I'll do that and see if they have any ideas.

@mmirus
Copy link
Contributor

mmirus commented Dec 2, 2022

FWIW, this version of the script might be a slightly more correct way of handling the arg transformation, but it doesn't seem like it makes any practical difference. 🤷‍♂️

#!/bin/bash

# Strip "/home/mmirus/git/repo/path/" from test paths sent to container
args=("${@/\/home\/mmirus\/git\/repo\/path\//}")
docker compose exec web bundle exec rspec "${args[@]}"

@mmirus
Copy link
Contributor

mmirus commented Dec 5, 2022

Some progress over on the neotest ticket. nvim-neotest/neotest#89 (comment)

@olimorris Does any of this spark ideas for how a more shippable solution could be constructed?

@pBorak
Copy link

pBorak commented Dec 5, 2022

@mmirus Tested out your 2nd solution. Works wonderfully!

edit: not sure if it's problem with the workaround or the neotest architecture itself but when I put binding.pry and try to debug while testing I cannot get into REPL 😕

edit 2: Just learned about require("neotest").run.attach(). It works!

@olimorris
Copy link
Owner

olimorris commented Dec 5, 2022

Some progress over on the neotest ticket. nvim-neotest/neotest#89 (comment)

@olimorris Does any of this spark ideas for how a more shippable solution could be constructed?

Nice work! This is super exciting.

I am more than happy to accept a PR (looks like you've done all the hard work already) to change how we call Neotest based on RSpec being run inside a Docker container. We could always pass a config flag or have a global variable.

@olimorris olimorris reopened this Dec 5, 2022
@mmirus
Copy link
Contributor

mmirus commented Dec 5, 2022

@olimorris I think the approach outlined in my latest comment is the one I prefer of the two I suggested in my comments over there.

If you agree, then no change is needed to the neotest-rspec code, unless you want to add a note to the README about how to set up the adapter to work with docker.

@olimorris
Copy link
Owner

@mmirus sorry I didn't see it. Whilst there's not direct support for it in neotest yet, I'd prefer to add it as a note in the readme in the usage section if you'd like to PR it

@mmirus
Copy link
Contributor

mmirus commented Dec 5, 2022

No need to apologize! I posted it after your last comment here. I'll send up a PR for the README sometime soon.

@olimorris olimorris pinned this issue Feb 23, 2023
@olimorris
Copy link
Owner

Big shoutout to @ramblex for the latest PR which adds support for Docker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants