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

EZP-20057: Merge Service tests into integration tests #1878

Merged
merged 23 commits into from
Jan 25, 2017

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Jan 9, 2017

Status: Ready for a review
Fixes: EZP-20057

TODO:

  • Remove obsolete Service integration tests (6ddfddc).
  • Fix found bugs in related integration tests (af01f8b).
  • Add missing TrashItem::$path property (1e5528b).
  • Check integration tests coverage for ContentTest*.
  • Check integration tests coverage for ContentTypeTest*.
  • Check integration tests coverage for LanguageTest*.
  • Check integration tests coverage for LocationTest*.
  • Check integration tests coverage for NameSchemaTest*.
  • Check integration tests coverage for ObjectStateTest*.
  • Check integration tests coverage for RepositoryTest*.
  • Check integration tests coverage for RoleTest*.
  • Check integration tests coverage for SearchTest*.
  • Check integration tests coverage for SectionTest*.
  • Check integration tests coverage for TrashTest*.
  • Check integration tests coverage for UserTest*.

* from the removed namespace \eZ\Publish\Core\Repository\Tests\Service\Integration.

@alongosz alongosz force-pushed the ezp-20057-remove-obsolete-tests branch 2 times, most recently from 72d28b0 to f2e320e Compare January 12, 2017 10:37
@alongosz
Copy link
Member Author

alongosz commented Jan 13, 2017

We need to be more careful when testing occurrences of exceptions :-)
For instance, if we test for InvalidArgumentException we should also use @expectedExceptionMessage to ensure checking for the proper exception.

An example: in af01f8b there are fixes for bugs related to this - test cases had mainLanguageCode and names struct fields missing, which also resulted in the InvalidArgumentException, but unrelated to what should actually be tested.

// cc @andrerom @pspanja @Nattfarinn

@alongosz alongosz force-pushed the ezp-20057-remove-obsolete-tests branch 3 times, most recently from 70faaca to 7e8bd0a Compare January 18, 2017 13:54
@alongosz alongosz force-pushed the ezp-20057-remove-obsolete-tests branch from 85525e7 to d6660cc Compare January 19, 2017 11:13
@alongosz alongosz changed the title [WIP] EZP-20057: Merge Service tests into integration tests EZP-20057: Merge Service tests into integration tests Jan 19, 2017
@alongosz alongosz requested a review from andrerom January 19, 2017 18:05
@alongosz
Copy link
Member Author

I looked over all the tests in \eZ\Publish\Core\Repository\Tests\Service\Integration namespace, improved proper integration tests and added some unit tests :)

Review ping @andrerom @Nattfarinn @pspanja

Copy link
Contributor

@andrerom andrerom left a comment

Choose a reason for hiding this comment

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

+1, congratulations on getting rid of 10k lines of code 🎉 👍

@alongosz alongosz requested a review from Nattfarinn January 20, 2017 09:47
@yannickroger
Copy link
Contributor

Impressive diff!

@andrerom
Copy link
Contributor

andrerom commented Jan 21, 2017

yeah, this was a long overdue (issue was created 12/Nov/12 4:47 PM..) cleanup to move integration tests that was part of unit tests into the actual integration tests. Small step to make it easier to work on repository :)

There is one thing here which is not test, but was uncovered here and that is: 1e5528b aligning TrashItem with Location on this.

@andrerom
Copy link
Contributor

andrerom commented Jan 25, 2017

As this thing is a needed cleanup, passing, not introducing new skips, and difficult to review I'm merging it. Thanks @alongosz

Merging using merge commit for keeping history given this tackles several things.

@andrerom andrerom merged commit 8c347fe into ezsystems:master Jan 25, 2017
@alongosz alongosz deleted the ezp-20057-remove-obsolete-tests branch January 25, 2017 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants