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

[messages - elixir] Convert proto-based messages to json schema based messages #1952

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -534,16 +534,16 @@ jobs:
paths:
- messages/elixir/lib/cucumber_messages/generated/

# gherkin-elixir:
# executor: docker-cucumber-build
# steps:
# - attach_workspace:
# at: "~/cucumber"
# - run:
# name: gherkin/elixir
# command: |
# cd gherkin/elixir
# make
gherkin-elixir:
executor: docker-cucumber-build
steps:
- attach_workspace:
at: "~/cucumber"
- run:
name: gherkin/elixir
command: |
cd gherkin/elixir
make

### PHP

Expand Down Expand Up @@ -737,9 +737,9 @@ workflows:
requires:
- prepare-parallel

# - gherkin-elixir:
# requires:
# - messages-elixir
- gherkin-elixir:
requires:
- messages-elixir

### PHP

Expand Down
14 changes: 6 additions & 8 deletions gherkin/elixir/lib/gherkin.ex
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ defmodule CucumberGherkin do
{:ok, binary} ->
hardcoded_mtype = "text/x.cucumber.gherkin+plain"
source_message = %Source{data: binary, uri: path, media_type: hardcoded_mtype}
source_envelope = %Envelope{message: {:source, source_message}}
source_envelope = %Envelope{source: source_message}
{:ok, source_envelope}

{:error, message} ->
{:error, "Could not read file. Got: #{message}"}
end
end

defp parse_messages(%Envelope{message: {:source, %Source{} = s}} = envelope, opts) do
defp parse_messages(%Envelope{source: %Source{} = s} = envelope, opts) do
%{messages: [], parsable?: true, source: s, ast_builder: nil}
|> add_source_envelope(envelope, opts)
|> add_gherkin_doc_envelope(opts)
Expand Down Expand Up @@ -119,7 +119,7 @@ defmodule CucumberGherkin do
parsed_meta <- parse_and_update_func.(),
{:only_ast, false, _} <- {:only_ast, no_ast?, parsed_meta},
%{parsable?: true} <- parsed_meta do
new_msg = put_msg_envelope(:gherkin_document, parsed_meta.ast_builder.gherkin_doc)
new_msg = %Envelope{gherkin_document: parsed_meta.ast_builder.gherkin_doc}
prepend_msg_to_meta(parsed_meta, new_msg)
else
{:skip?, true} -> meta
Expand All @@ -135,9 +135,9 @@ defmodule CucumberGherkin do
Enum.map(errors, fn error ->
message = CucumberGherkin.ParserException.get_message(error)
location = CucumberGherkin.ParserException.get_location(error)
source_ref = %CucumberMessages.SourceReference{location: location, reference: {:uri, uri}}
source_ref = %CucumberMessages.SourceReference{location: location, uri: uri}
to_be_wrapped = %CucumberMessages.ParseError{message: message, source: source_ref}
put_msg_envelope(:parse_error, to_be_wrapped)
%Envelope{parse_error: to_be_wrapped}
end)

{:error, result}
Expand All @@ -151,8 +151,6 @@ defmodule CucumberGherkin do
defp update_meta({:error, messages}, %{messages: m} = meta, :ast_builder),
do: %{meta | parsable?: false, messages: Enum.reduce(messages, m, &[&1 | &2])}

defp put_msg_envelope(type, m), do: %Envelope{message: {type, m}}

defp prepend_msg_to_meta(%{messages: m} = meta, new), do: %{meta | messages: [new | m]}

defp add_pickles_envelopes(%{ast_builder: builder, parsable?: true} = meta, opts) do
Expand All @@ -163,7 +161,7 @@ defmodule CucumberGherkin do
false ->
messages =
CucumberGherkin.PickleCompiler.compile(builder, meta.source.uri)
|> Enum.map(&put_msg_envelope(:pickle, &1))
|> Enum.map(fn pickle -> %Envelope{pickle: pickle} end)

%{meta | messages: List.flatten([messages | meta.messages])}
end
Expand Down
69 changes: 50 additions & 19 deletions gherkin/elixir/lib/gherkin/ast_builder/ast_builder.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@ defmodule CucumberGherkin.AstBuilder do
@moduledoc false
alias CucumberGherkin.{ParserContext, AstNode, Token, AstBuilderError, ParserException}
alias CucumberMessages.GherkinDocument.Comment, as: CommentMessage
alias CucumberMessages.GherkinDocument.Feature.Tag, as: MessageTag
alias CucumberMessages.GherkinDocument.Feature.Scenario, as: MessageScenario
alias CucumberMessages.GherkinDocument.Feature.Step, as: StepMessage
alias CucumberMessages.GherkinDocument.Feature.Step.DataTable, as: DataTableMessage
alias CucumberMessages.GherkinDocument.Feature.TableRow, as: TableRowMessage
alias CucumberMessages.GherkinDocument.Feature.TableRow.TableCell, as: TableCellMessage
alias CucumberMessages.GherkinDocument.Tag, as: MessageTag
alias CucumberMessages.GherkinDocument.Scenario, as: MessageScenario
alias CucumberMessages.GherkinDocument.Step, as: StepMessage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WannesFransen1994 is this document manually maintained? I see an inconsistency here: StepMessage where all the others are called Message*?

alias CucumberMessages.GherkinDocument.DataTable, as: DataTableMessage
alias CucumberMessages.GherkinDocument.TableRow, as: TableRowMessage
alias CucumberMessages.GherkinDocument.TableCell, as: TableCellMessage
alias CucumberMessages.GherkinDocument.Feature, as: FeatureMessage
alias CucumberMessages.GherkinDocument.Feature.FeatureChild, as: FeatureChildMessage
alias CucumberMessages.GherkinDocument.FeatureChild, as: FeatureChildMessage
alias CucumberMessages.GherkinDocument, as: GherkinDocumentMessage
alias CucumberMessages.GherkinDocument.Feature.Step.DocString, as: DocStringMessage
alias CucumberMessages.GherkinDocument.Feature.Background, as: BackgroundMessage
alias CucumberMessages.GherkinDocument.Feature.Scenario.Examples, as: ExamplesMessage
alias CucumberMessages.GherkinDocument.Feature.FeatureChild.Rule, as: RuleMessage
alias CucumberMessages.GherkinDocument.DocString, as: DocStringMessage
alias CucumberMessages.GherkinDocument.Background, as: BackgroundMessage
alias CucumberMessages.GherkinDocument.Examples, as: ExamplesMessage
alias CucumberMessages.GherkinDocument.Rule, as: RuleMessage

@me __MODULE__

Expand Down Expand Up @@ -58,7 +58,15 @@ defmodule CucumberGherkin.AstBuilder do
Comment ->
loc = Token.get_location(token)
comment_message = %CommentMessage{location: loc, text: token.line.content}
# TODO: Normally your struct should default to an empty list instead of nil.
# Due to the limited converter in the messages library, we make a case clause and "catch" this unexpected `nil` value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an option to make the converter smarter? It's nice not to have to maintain this workaround "forever".

# updated_comments =
# case builder.gherkin_doc.comments do
# nil -> [comment_message]
# list -> list ++ [comment_message]
# end
updated_comments = builder.gherkin_doc.comments ++ [comment_message]

updated_gherkin_doc = %{builder.gherkin_doc | comments: updated_comments}
updated_builder = %{builder | gherkin_doc: updated_gherkin_doc}
%{context | ast_builder: updated_builder}
Expand Down Expand Up @@ -281,9 +289,12 @@ defmodule CucumberGherkin.AstBuilder do
|> add_scen_def_children_to(scenarios)
|> tuplize(updated_context)
else
{:header?, _} -> nil
{:rule_line?, _} -> nil
|> tuplize(context)
{:header?, _} ->
nil

{:rule_line?, _} ->
nil
|> tuplize(context)
end
end

Expand Down Expand Up @@ -393,31 +404,51 @@ defmodule CucumberGherkin.AstBuilder do
defp add_mediatype_to(%DocStringMessage{} = m, d), do: %{m | media_type: d}

defp add_datatable_to(%StepMessage{} = m, nil), do: m
defp add_datatable_to(%StepMessage{} = m, d), do: %{m | argument: {:data_table, d}}
defp add_datatable_to(%StepMessage{} = m, d), do: %{m | data_table: d}

defp add_docstring_to(%StepMessage{} = m, nil), do: m
defp add_docstring_to(%StepMessage{} = m, d), do: %{m | argument: {:doc_string, d}}
defp add_docstring_to(%StepMessage{} = m, d), do: %{m | doc_string: d}

defp add_background_to(m, nil), do: m

defp add_background_to(%{__struct__: t} = m, d) when t in [FeatureMessage, RuleMessage] do
child = %FeatureChildMessage{value: {:background, d}}
child = %FeatureChildMessage{background: d}
# TODO: Normally your struct should default to an empty list instead of nil.
# Due to the limited converter in the messages library, we make a case clause and "catch" this unexpected `nil` value.
# case m.children do
# nil -> %{m | children: [child]}
# list -> %{m | children: list ++ [child]}
# end

%{m | children: m.children ++ [child]}
end

defp add_scen_def_children_to(%{__struct__: t} = m, scenario_definition_items)
when t in [FeatureMessage, RuleMessage] do
scenario_definition_items
|> Enum.reduce(m, fn scenario_def, feature_message_acc ->
child = %FeatureChildMessage{value: {:scenario, scenario_def}}
child = %FeatureChildMessage{scenario: scenario_def}
# TODO: Normally your struct should default to an empty list instead of nil.
# Due to the limited converter in the messages library, we make a case clause and "catch" this unexpected `nil` value.
# case feature_message_acc.children do
# nil -> %{feature_message_acc | children: [child]}
# list -> %{feature_message_acc | children: list ++ [child]}
# end
%{feature_message_acc | children: feature_message_acc.children ++ [child]}
end)
end

defp add_rule_children_to(%FeatureMessage{} = m, rule_items) do
rule_items
|> Enum.reduce(m, fn rule, feature_message_acc ->
child = %FeatureChildMessage{value: {:rule, rule}}
child = %FeatureChildMessage{rule: rule}

# TODO: Normally your struct should default to an empty list instead of nil.
# Due to the limited converter in the messages library, we make a case clause and "catch" this unexpected `nil` value.
# case feature_message_acc.children do
# nil -> %{feature_message_acc | children: [child]}
# list -> %{feature_message_acc | children: list ++ [child]}
# end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not seeing the case statement; I'm not all that great at elixir, but the code you describe really isn't there?

%{feature_message_acc | children: feature_message_acc.children ++ [child]}
end)
end
Expand Down
2 changes: 1 addition & 1 deletion gherkin/elixir/lib/gherkin/errors/unexpected_eof.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ defmodule CucumberGherkin.UnexpectedEOFError do
do:
struct!(CucumberGherkin.Token, line: me.line)
|> CucumberGherkin.Token.get_location()
|> Map.put(:column, 0)
|> Map.put(:column, nil)
end
end
95 changes: 61 additions & 34 deletions gherkin/elixir/lib/gherkin/pickle_compiler/pickle_compiler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,26 @@ defmodule CucumberGherkin.PickleCompiler do
defstruct id_gen: nil, pickles: [], language: nil, uri: nil

alias CucumberMessages.GherkinDocument.Feature, as: FeatureMessage
alias CucumberMessages.GherkinDocument.Feature.Scenario, as: ScenarioMessage
alias CucumberMessages.GherkinDocument.Feature.Step, as: StepMessage
alias CucumberMessages.GherkinDocument.Feature.TableRow, as: TableRowMessage
alias CucumberMessages.GherkinDocument.Scenario, as: ScenarioMessage
alias CucumberMessages.GherkinDocument.Step, as: StepMessage
alias CucumberMessages.GherkinDocument.TableRow, as: TableRowMessage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are inconsistent with ast_builder.ex where you use Message as a prefix. I think it's easier to understand the code base if you stick to the same naming rules. Or is there a specific reason to deviate here?

alias CucumberMessages.Pickle, as: PickleMessage
alias CucumberMessages.Pickle.PickleStep, as: PickleStepMessage
alias CucumberMessages.Pickle.PickleTag, as: PickleTagMessage
alias CucumberMessages.GherkinDocument.Feature.Tag, as: TagMessage
alias CucumberMessages.GherkinDocument.Feature.FeatureChild, as: FeatureChildMessage
alias CucumberMessages.GherkinDocument.Feature.FeatureChild.Rule, as: RuleMessage
alias CucumberMessages.GherkinDocument.Feature.Scenario.Examples, as: ExampleMessage
alias CucumberMessages.PickleStepArgument.PickleTable, as: PickleTableMessage
alias CucumberMessages.GherkinDocument.Tag, as: TagMessage
alias CucumberMessages.GherkinDocument.FeatureChild, as: FeatureChildMessage
alias CucumberMessages.GherkinDocument.Rule, as: RuleMessage
alias CucumberMessages.GherkinDocument.Examples, as: ExampleMessage
alias CucumberMessages.Pickle.PickleTable, as: PickleTableMessage

alias CucumberMessages.PickleStepArgument.PickleTable.PickleTableRow.PickleTableCell,
alias CucumberMessages.Pickle.PickleTableCell,
as: PickleTableCellMessage

alias CucumberMessages.PickleStepArgument.PickleTable.PickleTableRow, as: PickleTableRowMessage
alias CucumberMessages.GherkinDocument.Feature.Step.DataTable, as: DataTableMessage
alias CucumberMessages.Pickle.PickleTableRow, as: PickleTableRowMessage
alias CucumberMessages.GherkinDocument.DataTable, as: DataTableMessage

alias CucumberMessages.Pickle.PickleDocString, as: PickleDocStringMessage
alias CucumberMessages.GherkinDocument.DocString, as: DocStringMessage

@me __MODULE__

Expand All @@ -44,16 +47,27 @@ defmodule CucumberGherkin.PickleCompiler do
}

Enum.reduce(f.children, meta_info, fn child, m_acc ->
case child.value do
{:background, bg} ->
%{m_acc | feature_backgr_steps: bg.steps}
cond do
child.background != nil ->
%{m_acc | feature_backgr_steps: child.background.steps}

{:rule, rule} ->
compile_rule(m_acc, rule)
child.rule != nil ->
compile_rule(m_acc, child.rule)

{:scenario, s} ->
compile_scenario(m_acc, s, :feature_backgr_steps)
child.scenario != nil ->
compile_scenario(m_acc, child.scenario, :feature_backgr_steps)
end

# case child.value do
# {:background, bg} ->
# %{m_acc | feature_backgr_steps: bg.steps}

# {:rule, rule} ->
# compile_rule(m_acc, rule)

# {:scenario, s} ->
# compile_scenario(m_acc, s, :feature_backgr_steps)
# end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the value of this comment block? It looks like it's preserving the old code, but once we're moved over, we have the old code in Git for historic reference. No need to keep a copy?

end)
end

Expand All @@ -62,10 +76,10 @@ defmodule CucumberGherkin.PickleCompiler do
rule_tags = meta_info.feature_tags ++ r.tags

Enum.reduce(r.children, resetted_meta_info, fn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was already there, but reset is the present perfect tense of to reset (i.e. not resetted)

%FeatureChildMessage{value: {:background, bg}}, m_acc ->
%FeatureChildMessage{background: bg}, m_acc when not is_nil(bg) ->
%{m_acc | rule_backgr_steps: m_acc.rule_backgr_steps ++ bg.steps}

%FeatureChildMessage{value: {:scenario, s}}, m_acc ->
%FeatureChildMessage{scenario: s}, m_acc when not is_nil(s) ->
%{m_acc | feature_tags: rule_tags} |> compile_scenario(s, :rule_backgr_steps)
end)
end
Expand Down Expand Up @@ -213,14 +227,12 @@ defmodule CucumberGherkin.PickleCompiler do
%PickleTableMessage{rows: table_row_messages}
end

alias CucumberMessages.PickleStepArgument.PickleDocString, as: PickleDocStringMessage
alias CucumberMessages.GherkinDocument.Feature.Step.DocString, as: DocStringMessage

defp pickle_doc_string_creator(%DocStringMessage{} = d, variable_cells, value_cells) do
content = interpolate(d.content, variable_cells, value_cells)

media_type =
case d.media_type do
nil -> nil
"" -> ""
media_type -> interpolate(media_type, variable_cells, value_cells)
end
Expand All @@ -237,34 +249,49 @@ defmodule CucumberGherkin.PickleCompiler do
defp add_ast_node_id(%PickleStepMessage{ast_node_ids: ids} = m, %TableRowMessage{} = row),
do: %{m | ast_node_ids: ids ++ [row.id]}

defp add_datatable(%PickleStepMessage{} = m, %StepMessage{argument: nil}, _, _), do: m
defp add_datatable(
%PickleStepMessage{} = m,
%StepMessage{doc_string: nil, data_table: nil},
_,
_
),
do: m

defp add_datatable(%PickleStepMessage{} = m, %StepMessage{argument: {:doc_string, _}}, _, _),
defp add_datatable(%PickleStepMessage{} = m, %StepMessage{doc_string: ds}, _, _) when ds != nil,
do: m

defp add_datatable(
%PickleStepMessage{} = m,
%StepMessage{argument: {:data_table, d}},
%StepMessage{data_table: d},
variable_cells,
value_cells
) do
)
when d != nil do
result = pickle_data_table_creator(d, variable_cells, value_cells)
%{m | argument: result}
%{m | argument: %{data_table: result}}
end

defp add_doc_string(%PickleStepMessage{} = m, %StepMessage{argument: nil}, _, _), do: m
defp add_doc_string(
%PickleStepMessage{} = m,
%StepMessage{doc_string: nil, data_table: nil},
_,
_
),
do: m

defp add_doc_string(%PickleStepMessage{} = m, %StepMessage{argument: {:data_table, _}}, _, _),
do: m
defp add_doc_string(%PickleStepMessage{} = m, %StepMessage{data_table: dt}, _, _)
when dt != nil,
do: m

defp add_doc_string(
%PickleStepMessage{} = m,
%StepMessage{argument: {:doc_string, d}},
%StepMessage{doc_string: d},
variable_cells,
value_cells
) do
)
when d != nil do
result = pickle_doc_string_creator(d, variable_cells, value_cells)
%{m | argument: result}
%{m | argument: %{doc_string: result}}
end

defp get_id_and_update_compiler_acc(%@me{id_gen: gen} = compiler_acc) do
Expand Down
Loading