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

names in Test::Case #82

Merged
merged 3 commits into from
Feb 13, 2015
Merged

Conversation

richarda
Copy link
Contributor

@richarda richarda commented Feb 2, 2015

This PR starts working on cucumber issue #768 where we want a name like "scenario name" not "Scenario: scenario name"

@mattwynne and @akostadinov, let me know if this is the direction you were thinking. There were two other requests in the issue about wanting values from an example table and a separate method to get row numbers, but I'm not entirely sure what the end goal is.

How would these details be used? ... Can you give me an example? 😄

I'm not completely sure just sticking on extra results to the NameBuilder is the way to go either, but this does give the simplified name now.

@akostadinov
Copy link

I'm not sure about method names. It makes IMO more sense to have #name return the "simple_name" in your PR but either way works for me.

wrt file/row number, we use this information to link between scenarios/outlines to an external bug tracker. Would be nice to have it in the public API. Ruby is nice when you need to hack around APIs but IMO it's nice to have it officially supported.

@mattwynne
Copy link
Member

I think idea for the request for file / row number was to make them available on other properties of the Test::Case. So we can do that under another ticket.

@mattwynne
Copy link
Member

I think I'd be happy to just lose the old functionality and have what's in #simple_name replace what's in #name. We can add something like that back in if we find we need it. Can you do that @richarda?

@richarda
Copy link
Contributor Author

richarda commented Feb 2, 2015

I can try to do that @mattwynne. I'll start implementing some changes so #name will return "scenario name" and not "Scenario: scenario name".
The reason I didn't change #name initially is that many tests depend on a specific text output provided by the existing implementation, and I wasn't sure if there were external dependencies on the more detailed name that #name currently provides.

@mattwynne
Copy link
Member

I think the tests were just leaning on that output to determine the order of things coming out IIRC. We might break the odd thing in the front-end cucumber gem, but we can fix those as we find them.

@richarda
Copy link
Contributor Author

richarda commented Feb 2, 2015

That makes sense. I'll do it. Would it make sense to run the 'cucumber' test suite against any changes here? Do I just point the Gemspec at my local repo of cucumber-ruby-core?

@mattwynne
Copy link
Member

Yes, it would be great to do that too. As long as you keep the two projects side-by-side it should just pick up your local core anyway

@richarda
Copy link
Contributor Author

richarda commented Feb 3, 2015

So this change to Test::Case#name does break a lot of features in the main cucumber repo (19 scenarios). I just wanted to make sure this output here is what we are looking to get. If so, I'll take a look at the failing scenarios in cucumber and get a PR going for that too.

Very slick library dependency @mattwynne! Being able to run the cucumber features so easily against my local cucumber-ruby-core made my evening 👍

@mattwynne
Copy link
Member

Yes, the output in this PR looks good to me.

@mattwynne
Copy link
Member

One thing we haven't worked out with the build system is how to have it reflected on travis, so if you open up a PR in cucumber pointing to a dev version of the core it will fail on travis / GH even if it's working on your machine. If you have any ideas about how to fix that I'd love to hear them.

@richarda
Copy link
Contributor Author

richarda commented Feb 4, 2015

If you had a PR for cucumber with the same branch name as a PR in cucumber-ruby-core, a variable in rake could get the cucumber-ruby-core gem from github on the specified branch, if it existed?
Then the PR in cucumber would pass on Travis, but once it go merged in, the corresponding PR on cucumber-ruby-core wold need to be merged in so the master branch build on Travis would pass.
I'm optimistic that would be possible with rake, but I haven't tried yet.

@richarda
Copy link
Contributor Author

richarda commented Feb 6, 2015

It looks like at least a few, if not all the feature failures in the cucumber gem are about the scenario name. The output that is currently expected is something like this:

Failing Scenarios:
      cucumber features/test_feature_1.feature:2 # Scenario: Test Scenario 1

Should I just change the expected output to this?

Failing Scenarios:
      cucumber features/test_feature_1.feature:2 # Test Scenario 1

That seems reasonable. It's listed as a "Failing Scenario", with a line number for the feature file, and the name of the scenario. It actually makes the output a little more compact.

Alternatively, we could add the "Scenario: " string to the value returned by #name if it's important to maintain the existing behavior.

I'm going to work on changing the expect output for now.

@richarda
Copy link
Contributor Author

richarda commented Feb 6, 2015

@mattwynne, I added a PR to cucumber that changes the expected output to match what this PR will provide. As expected, it's failing the build because of the dependency, but when I run both branches locally everything is green.

mattwynne added a commit that referenced this pull request Feb 13, 2015
Test::Case#name no longer includes keyword
@mattwynne mattwynne merged commit b7ab0f5 into cucumber:master Feb 13, 2015
@akostadinov
Copy link

How can I know when a new beta is released?

@mattwynne
Copy link
Member

Good question.

It should be announced in the following places:

Are there any other ways you can think of that we could easily bring a new release to your attention?

@mattwynne
Copy link
Member

I guess we could keep this bug open under a 2.0.0.rc.4 milestone and then close it when we release it, but I tend to favour closing bugs once they've been resolved in the codebase, so I can see which ones still need working on.

@mattwynne
Copy link
Member

@akostadinov
Copy link

Thanks! I've just installed and will try it out.

@richarda richarda deleted the 768-scenario-name branch February 14, 2015 02:50
@lock
Copy link

lock bot commented Oct 24, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants