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

Added Behat integration #128

Merged
merged 3 commits into from
Oct 30, 2020
Merged

Added Behat integration #128

merged 3 commits into from
Oct 30, 2020

Conversation

wouterj
Copy link
Contributor

@wouterj wouterj commented Sep 18, 2020

This PR adds the necessary classes to integrate with Behat, similar to the existing PHPunit integration. I hope this is something you want to add, it would be of great help for our test suite :)

If you want this, do you want me to add a Behat test set-up to functionally test the integration?

@wouterj wouterj changed the title Added Behat integration [WIP] Added Behat integration Sep 18, 2020
@wouterj wouterj force-pushed the feature/behat branch 2 times, most recently from de17104 to 41a3add Compare September 18, 2020 14:36
@wouterj wouterj changed the title [WIP] Added Behat integration Added Behat integration Sep 18, 2020
Copy link
Owner

@dmaicher dmaicher left a comment

Choose a reason for hiding this comment

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

Thanks for proposing this @wouterj 👍

Having this covered by a functional test would be nice. Will this require a lot of code?

README.md Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
@wouterj
Copy link
Contributor Author

wouterj commented Sep 21, 2020

Having this covered by a functional test would be nice. Will this require a lot of code?

I'll have a look, I think it'll have the same or less code as the PHPunit functional test.

Thanks for your review, I'll try to find some time this week to work on this PR.

@wouterj
Copy link
Contributor Author

wouterj commented Oct 4, 2020

Updated the PR, I've added functional behat tests (similar to those existing in PHPunit). I couldn't understand what PhpunitTest::testRollBackChangesWithReOpenedConnection was testing, so I haven't copied this one to the Behat tests. I also didn't move the testPreviousChangesAreRolledBack tests to Behat, it's a bit non-behat-ish to do so. As all scenarios start with Given there are 0 rows, we already test it enough imho.

Please note that I also moved files around, so the diff is a big ugly. The directory structure now is:

tests/
  DAMA/
    ...
  Functional/
    app/                         # symfony application files
      AppKernel.php
      config.yml
      parameters.yml
      parmeters.yml.dist
    features/                    # behat features
      bootstrap/
        FeatureContext.php
      behat_integration.feature
    FunctionalTestTrait.php      # shared functional test methods
    PhpunitTest.php              # phpunit tests
  behat.yml
  bootstrap.php
  phpunit.xml
  phpunit7.xml

Copy link
Owner

@dmaicher dmaicher left a comment

Choose a reason for hiding this comment

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

Nice 👍 Thanks

@dmaicher
Copy link
Owner

dmaicher commented Oct 7, 2020

@mnapoli @dkarlovi what do you think about this? Any chance you could review the behat part? Since you commented here I guess you are using this bundle with behat? 😊

@dmaicher
Copy link
Owner

@wouterj I added some changes on master (php8 support for example). Could you solve the conflicts please? I would like to merge this as well for release 6.4.0

@wouterj wouterj force-pushed the feature/behat branch 2 times, most recently from 5381d1d to b9e0a15 Compare October 30, 2020 09:44
Copy link
Owner

@dmaicher dmaicher left a comment

Choose a reason for hiding this comment

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

Thanks @wouterj 👍

@dmaicher dmaicher merged commit a032792 into dmaicher:master Oct 30, 2020
@wouterj wouterj deleted the feature/behat branch October 30, 2020 10:37
@wouterj
Copy link
Contributor Author

wouterj commented Oct 30, 2020

Thank you for your time reviewing & merging this PR @dmaicher!

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