cabbage-ex / cabbage

Story BDD tool for executing elixir in ExUnit
MIT License
138 stars 33 forks source link

BUG: import_feature fails to import module without tags #17

Closed hisapy closed 7 years ago

hisapy commented 7 years ago

Hi,

First of all, thanks for this project. I was already doing stuff with ExUnit and .feature files but manually.

Now, regarding the bug:

Given a module with steps like

defmodule Webapp.CommonFeatureSteps do
  use Cabbage.Feature
  # TODO: match role into a variable instead of hardcoding Admin
  defgiven ~r/^login as  "Admin"$/, _vars, %{session: session} do
    # some login logic here
  end
  defand ~r/^visit "(?<path>.*?)"$/, %{path: path}, %{session: session} do
     # some visit logic here
  end
end

When I import_feature Webapp.CommonFeatureSteps then I get the following error:

** (Protocol.UndefinedError) protocol Enumerable not implemented for nil
    (elixir) lib/enum.ex:1: Enumerable.impl_for!/1
    (elixir) lib/enum.ex:116: Enumerable.reduce/3
    (elixir) lib/enum.ex:1776: Enum.reduce/3

I traced the error and in this case, with a module with steps but without tags, it's because in

for {name, block} <- unquote(module).raw_tags() do
  Cabbage.Feature.Helpers.add_tag(__MODULE__, name, block)
end

inside defmacro import_feature in Cabbage.Feature, the value of unquote(module).raw_tags() is nil.

As a side note, I think it might read better import_steps and import_tags instead of import_feature.

mgwidmann commented 7 years ago

This should be fixed now with the latest release, please give it a try and let me know if there are any issues!

hisapy commented 7 years ago

Working ok now 👍

mgwidmann commented 7 years ago

Must have missed your suggestion to break out import_feature into import_steps and import_tags... Thats not a bad idea, perhaps we should have import_feature simply do both import_steps and import_tags. I'd appreciate a PR if you have the time.

hisapy commented 7 years ago

Hi @mgwidmann I'd love to PR but right now I'm on deadline everywhere ... maybe we can create an issue to address this later in a few weeks?

mgwidmann commented 7 years ago

I've added a new issue for this, thanks!