Skip to content

improve test reports for cucumber-android #598

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

Merged
merged 5 commits into from
Oct 12, 2013

Conversation

SierraGolf
Copy link
Contributor

This PR is a follow up for "extension of the Formatter interface for more precise lifecycle handling" and will not compile as long as the other PR is not merged.

This PR is a refactoring of the previously quite buggy AndroidReporter. The following bugs have been fixed:

  • double entries for end tags
  • one to many entry start/end tag for outlines
  • green tests for exceptions coming from hooks
  • android maven plugin exceptions for undefined steps
  • wrong count of scenario examples
  • wrong step associated with missing step definition

In order to test the behaviour properly three new test dependencies have been added. Additionally the project structure was changed to the usual maven structure.

Questions/Problems:

  1. Which tests should I extend in order consider the newly added Formatter methods?
  2. In general the android module builds and also the whole project builds, when skipping tests. However there are weird tests errors which differ depending on which java version is used, when trying to build the whole project. With 1.6.0_51 I get the following errors:
Tests in error:
  URLOutputStreamTest.throws_ioe_if_http_response_is_500:109 � Connect Connectio...
  URLOutputStreamTest.throws_fnfe_if_http_response_is_404:85 � Connect Connectio...
  URLOutputStreamTest.can_http_put:76 � Connect Connection refused

and JSONPrettyFormatterTest.featureWithOutlineTest fails: because the expected out contains the before hook information in the wrong scenario. When I run with 1.7.0_21 the errors disappear, but the failure remains.

@friederbluemle
Copy link
Contributor

Looks like a great improvement! Any chance this could get merged soon?
Just a minor suggestion for the future: Please use separate commits for things like restructuring the project and making actual code changes. Otherwise it is hard to see what has changed.
Thanks.

@SierraGolf
Copy link
Contributor Author

Just a minor suggestion for the future: Please use separate commits for things like restructuring the project and making actual code changes. Otherwise it is hard to see what has changed.

Yeah, I am sorry for that. The restructuring happened somewhere half way through.

…roidReporter

* restructured project to be more maven compliant
* added dependencies for testing purposes
@brasmusson
Copy link
Contributor

@friederbluemle Unfortunately this PR build on a PR for Gherkin, and (as far as I know) snapshot releases are not deployed for Gherkin, so this PR cannot be merges until after the next Gherkin release.

@mfellner
Copy link
Contributor

mfellner commented Oct 6, 2013

Note: depends on cucumber/gherkin#275.

@SierraGolf
Copy link
Contributor Author

Answer to my second question: the commit cucumber/gherkin@b425544 changed the output of the JsonFormattter (to the better) which requires changing the JSONPrettyFormatterTest.json in the cucumber-core module.

@aslakhellesoy Do you want me to include the fix for the failing test in this PR?

@brasmusson
Copy link
Contributor

@SierraGolf The needed change in the JsonFormatterTest due to the change in cucumber/gherkin@b425544 is taken care of in #570, which supplies the failing test to drive cucumber/gherkin#270 (which the commit belongs to), so it is not necessary for you to fix it in this PR. But if you what a passing build together with Gherkin built from the latest source, you would also need to include #570.

@mfellner
Copy link
Contributor

mfellner commented Oct 6, 2013

@SierraGolf, I think there is another general problem with the expected output in JSONPrettyFormatterTest.json. As of now, a "before" element is included in these JSON objects:

  • "id": "feature-3;scenario-1"
  • "id": "feature-3;scenariooutline-1"
  • "id": "feature-3;scenariooutline-1;;2"
  • "id": "feature-3;scenariooutline-1;;3"

However I believe the element should be placed in these objects instead:

  • "id": "feature-3;scenario-1"
  • "id": "feature-3;scenariooutline-1;;1"
  • "id": "feature-3;scenariooutline-1;;2"
  • "id": "feature-3;scenariooutline-1;;3"
  • "id": "feature-3;scenario-2"

Note that "id": "feature-3;scenariooutline-1;;1" is not even included in the JSON at all, even though it is referenced in "id": "feature-3;scenariooutline-1;".

In b425544abc04821be2319d692aebb7d67c3d5daf @brasmusson noted where the hooks should be placed.

Update: Ok, I just realized this is at least partially fixed in #570. But "id": "feature-3;scenariooutline-1;;1" is still missing, why?

@SierraGolf
Copy link
Contributor Author

@brasmusson thanks for the info. in case #570 is merged before this one, I will rebase. I probably need to update this PR anyways as soon as the gherkin project is released.

@aslakhellesoy
Copy link
Contributor

I tried building this branch (after first updating the gherkin dependency to 2.12.2.

The cucumber-core module has one failing test, JSONPrettyFormatterTest.featureWithOutlineTest:35.

Can you take a look at that, @SierraGolf ?

@brasmusson
Copy link
Contributor

@aslakhellesoy You need both #570 and this PR to get a passing build with Gherkin 2.12.2. #570 because of cucumber/gherkin#270 and this PR because of cucumber/gherkin#275

@aslakhellesoy
Copy link
Contributor

@brasmusson thanks! that seems to work.

aslakhellesoy added a commit that referenced this pull request Oct 12, 2013
@aslakhellesoy aslakhellesoy merged commit 7bf5074 into cucumber:master Oct 12, 2013
@lock
Copy link

lock bot commented Oct 25, 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 25, 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.

5 participants