pezra / rspec-mode

An RSpec minor mode for Emacs
258 stars 112 forks source link

Setting docker variables, does nothing #178

Closed fibrasek closed 4 years ago

fibrasek commented 6 years ago

Hello,

I have setup the variables to run via docker, but it seems to do nothing, here is my config:

(use-package rspec-mode
  :ensure t
  :init (setq rspec-use-rvm t
              rspec-use-docker-when-possible t
              rspec-docker-command "docker exec"
              rspec-docker-cwd "/b2beauty/beautydate/"
              rspec-docker-container "beautydate_web_1"))

And the output from C-c , v:

-*- mode: rspec-compilation; default-directory: "~/Dev/BeautyDate/beautydate/" -*-
RSpec Compilation started at Mon Sep 10 12:28:45

bundle exec rspec --format documentation /home/fibrasek/Dev/BeautyDate/beautydate/spec/models/sales_order_spec.rb

Am I doing something wrong? If so, what?

dgutov commented 6 years ago

Is there a Dockerfile in the project root?

fibrasek commented 6 years ago

@dgutov yes, but they end with different extensions, eg.: Dockerfile.aws

dgutov commented 6 years ago

How do you choose which one to use?

fibrasek commented 6 years ago

Locally its used via docker-compose

dgutov commented 6 years ago

What is?

fibrasek commented 6 years ago

You can read about docker-compose here

micdahl commented 6 years ago

It is because there is a function which searches for "Dockerfile" to determine the projects root directory. If there is none, the use of docker is disabled. This was intended to enable docker-compose from the root directory of the project, if some files deeper in the directory tree are beeing edited when rspec mode is run. Even the projects root directory is needed to get the relative filename of specs to run. As a quick workaround you can create an empty Dockerfile in your apps root directory.

To resolve the issue, there could be a distinction if docker or docker-compose is given as rspec-docker-command. Also it would be reasonable to look for "docker-compose.yml" instead of "Dockerfile" and/or to allow a variable filename to find projects root.

fibrasek commented 5 years ago

@micdahl seems a reasonable workaround. Extending the topic, which function looks for the Dockerfile in the project root? Maybe extending how it searches for the file could solve the issue of having multiple Dockerfiles with extensions.

micdahl commented 5 years ago

After revisiting, I found the responsible code here. I think the second part of the and statement could just be removed, so that rspec-docker-p only relies on rspec-use-docker-when-possible:

(defun rspec-docker-p ()
  rspec-use-docker-when-possible)

That should do the trick, but I am unsure about the correct use of lisp, as I haven't had a look on it for years.

dgutov commented 5 years ago

@Fibrasek You can certainly use advice on rspec-docker-p to make it behave however you like.

@micdahl Not sure simply removing the existence check is the correct approach. After all, all other similar variables work that way. At the very least, the option would have to be renamed.

fibrasek commented 5 years ago

@dgutov thanks for the tip, Ill look up on how to properly use advice, since my lisp skills are pretty basic :smile:

nfedyashev commented 4 years ago

I believe that it is worth mentioning in README that rspec-mode expects to find Dockerfile in the root directory. For new users it is very confusing because having Dockerfile in non-root directory is pretty common practice(for example https://github.com/evilmartians/terraforming-rails/tree/master/examples/dockerdev ).

Other than that rspec-mode is awesome

dgutov commented 4 years ago

@nfedyashev How can we support this use case? Can we just look for docker-compose.yml as well?

nfedyashev commented 4 years ago

@dgutov

Can we just look for docker-compose.yml as well?

yes or checking for the presence of files named like docker-compose* - it covers cases like

docker-compose.yml docker-compose.consul.yml docker-compose.dev.yml docker-compose.prod.yml docker-compose.override.yml docker-compose.sec.yml docker-compose.storage.yml docker-compose.sync.yml docker-compose.development.yml

Examples were taken from https://github.com/search?q=docker-compose&type=Code

dgutov commented 4 years ago

I think most of those are used for scripting purposes, don't they? Like, the CI script copies docker-compose.gitlab.yml to docker-compose.yml before running the build.

In any case, I can simply add a custom var with the file name to look for.

nfedyashev commented 4 years ago

Although I never used custom docker-compose yaml confg files(like docker-compose.custom.yml) I suspect some may use it for expressiveness/explicitness in their complex projects.

CI script most likely doesn't need to copy/rename docker-compose config files because it can explicitly specify which config to pick up using docker-compose -f docker-compose.custom.yml

In any case, I can simply add a custom var with the file name to look for.

:+1: and the default value would be "docker-compose.yml"? This should solve the issue for most generic projects and make it really easy to understand what to do in the case of custom Compose files because it is explicit and clear in form of variable name.

dgutov commented 4 years ago

and the default value would be "docker-compose.yml"?

Why not "Dockerfile"? :)

In your experience, do people use Docker without docker-compose these days?

pedz commented 4 years ago

I have never used docker compose but I believe I’m the minority.

How about a list?

On Dec 6, 2019, at 9:02 AM, Dmitry Gutov notifications@github.com wrote:

 and the default value would be "docker-compose.yml"?

Why not "Dockerfile"? :)

In your experience, do people use Docker without docker-compose these days?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

dgutov commented 4 years ago

I'm, uh, trying to minimize the number of disk accesses in the default config (mostly important when one uses Tramp, but still). The var could be a list, but the default would have one element.

Speaking of, one of the recent PRs sneaked in a change in the default of rspec-docker-command. @pedz, did you have to customize that?

pedz commented 4 years ago

I’ve not used Docker with Ruby On Rails. I was using it with Python.

Given that this is Docker + RoR + Development (since this is using Rspec) … I’d go with docker compose.

I’m very curious to explore this. I spent two weeks exploring using Virtual Box, Vagrant, and Arch Linux just so I didn’t have to install (evil) Chrome on my laptop. Turns out you can use Brave instead of Chrome.

On Dec 6, 2019, at 9:24 AM, Dmitry Gutov notifications@github.com wrote:

I'm, uh, trying to minimize the number of disk accesses in the default config (mostly important when one uses Tramp). The var could be a list, but the default would have one element.

Speaking of, one of the recent PRs sneaked in a change in the default of rspec-docker-command. @pedz https://github.com/pedz, did you have to customize that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pezra/rspec-mode/issues/178?email_source=notifications&email_token=AAASST4VCY6GGQAMX6QHVT3QXJVDHA5CNFSM4FUGTBT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGENKIA#issuecomment-562615584, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAASST6IL4FX22E7HUKH6A3QXJVDHANCNFSM4FUGTBTQ.

dgutov commented 4 years ago

Given that this is Docker + RoR + Development (since this is using Rspec) … I’d go with docker compose

Excellent, docker-compose it is!

Turns out you can use Brave instead of Chrome

Or, um, Firefox?

nfedyashev commented 4 years ago

@dgutov

and the default value would be "docker-compose.yml"? Why not "Dockerfile"? :)

oh, sorry, I might have misunderstood your point. I thought that's going to be combined check(presence of Dockerfile OR docker-compose.yml).

In your experience, do people use Docker without docker-compose these days?

I guess some people might be still using Docker without docker-compose.

dgutov commented 4 years ago

some people might be still using Docker without docker-compose

Then they have to customize docker-command anyway.