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

Languages feature #12

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

regnisolbap
Copy link

Hi there,

As I noticed in #11, I couldn't find how to use gherkin with some other languages than english. I'm just starting my Elixir journey so this PR is surely not perfect, at least the tests are passing and it seems to work well (I tested it only in french and spanish).

I've just created a Keywords module which holds a struct and some functions to interact with keywords loaded from a gherkin-languages.json file at the root of the project (a script to automatically download this file from the official gherkin repo everytime this project gets published could be a good thing to add). Then I updated the parsers to handle those keywords instead of just plain strings in english.

Anyway, let me know what you think about it !

@revati
Copy link
Collaborator

revati commented Jul 31, 2019

Nice work, there is more change than I anticipated tho
Hope I will be able to look into this in weekend

{
"af": {
"and": [
"* ",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I also noticed this in gherkin documentation: https://cucumber.io/docs/gherkin/reference/#gherkin-dialects
what is meant by *. Is it like a wild card, catch-all?
in that case Given, When, Then, And, But all are equal. And contextually there is no difference (this note probably is more related to cabbage library on how to handle step definitions and how important it is to use correct step type. It seems that there is 0 importance to it, only easier to read for the eye because is more similar to plain English)

Copy link
Author

@regnisolbap regnisolbap Aug 1, 2019

Choose a reason for hiding this comment

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

I'm really not a gherkin expert, but yes it seems like it can be used to avoid redundancy in feature files (cf https://stackoverflow.com/a/27363033). I removed it from the keywords list in Keywords.cleanup_list/1 but maybe it's better to leave it there, what do you think ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that the primary concern for this library should be compatibility with original spec. So if gherkin/cucumber supports it, we should as well. So * should be supported.

@revati
Copy link
Collaborator

revati commented Aug 9, 2019

What we should do I think, is to create some base performance test suite. Which we can run, on the current implementation and yours and use it as a reference in future, to see how code changes impacts library performance

@regnisolbap
Copy link
Author

Hey @revati, yes some performance tests might be nice but I really don't know where to start with it... if you have some guidelines I'll be glad to help on this !

@revati
Copy link
Collaborator

revati commented Aug 14, 2019

Yes, I joined the cucumber slack channel and they pointed out that there is https://github.com/cucumber/cucumber/tree/master/gherkin/testdata which can be used as

  1. test odd edge cases that are provided there (there is also expected json output to compare with)
  2. parse all those feature files and see how good this lib performs.

I will set up a seperate branch where will be benchee base, so you will just need to merge it in and run command

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

Successfully merging this pull request may close these issues.

2 participants