Skip to content

Add integration test for search by catalog in user collection #1107

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 7, 2019
Merged

Add integration test for search by catalog in user collection #1107

merged 1 commit into from
Aug 7, 2019

Conversation

mukeshk
Copy link
Contributor

@mukeshk mukeshk commented Aug 6, 2019

Addressed to #1097

@mukeshk mukeshk requested a review from php-coder as a code owner August 6, 2019 06:11
@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 6, 2019

The test cases which I have added works.
When I ran the entire test suite, around 8 to 10 test cases broke down.
I need to investigate why the test cases are broken.

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch from 4e2cf5e to f7ead33 Compare August 6, 2019 06:16
@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 6, 2019

Create the WIP to discuss the issue. Thanks

@mystamps-bot
Copy link

mystamps-bot commented Aug 6, 2019 •
edited
Loading

1 Message
📖 @mukeshk thank you for the PR! All quality checks have been passed! Next step is to wait when @php-coder will review this code

Generated by 🚫 Danger

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch from f7ead33 to e6f7d2c Compare August 6, 2019 06:28
@php-coder
Copy link
Owner

When I ran the entire test suite, around 8 to 10 test cases broke down. I need to investigate why the test cases are broken.

I see that the tests don't fail on Travis CI, so it must be something related to your environment. If you provide more details, perhaps, I can help with debugging.

@php-coder
Copy link
Owner

@mukeshk Thank you for the PR! I'll review it during the day!

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch from e6f7d2c to 1515fc8 Compare August 6, 2019 10:47
@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 6, 2019

I had the application running in one console.
And in another console, I was running the robot framework test suite.
I ran it a couple of times without restarting the app server.

There were some JDBC exceptions for unique constraints.

@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 6, 2019

Can you review ?

@php-coder
Copy link
Owner

There were some JDBC exceptions for unique constraints.

Then, I'd say, that it works as designed. Some cases modify database and they can't be run more than once on the same instance of database. When we restart the app, the database is also get recreated.

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch 2 times, most recently from c08115e to 53fde6b Compare August 6, 2019 16:14
@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 6, 2019

comments addressed.

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch from 53fde6b to 926e2bc Compare August 6, 2019 16:30
@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch 2 times, most recently from 5b8dde4 to d2db7a8 Compare August 6, 2019 17:07
@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 6, 2019

address the comments

@mukeshk mukeshk changed the title [WIP] Add integration test for search by catalog in user collection Add integration test for search by catalog in user collection Aug 6, 2019
@php-coder
Copy link
Owner

Could you, please, modify commit message to be:

test(series/search/logic-user.robot): add tests for searching the series in user's collection.

Related to #673
Fix #1097

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch 2 times, most recently from c55966e to bc7f4bf Compare August 7, 2019 05:26
@php-coder
Copy link
Owner

Could you also remove the following comment?

mystamps/src/main/webapp/WEB-INF/views/site/index.html

Line 185 in bea703f

<!-- @todo #673 Add integration tests for search by a catalog in user's collection -->

@mukeshk mukeshk force-pushed the gh1097_search_by_catalog_in_user_collection branch from bc7f4bf to 1133026 Compare August 7, 2019 12:32
@mukeshk
Copy link
Contributor Author

mukeshk commented Aug 7, 2019

removed the todo

@php-coder php-coder merged commit 7aaa69e into php-coder:master Aug 7, 2019
@php-coder
Copy link
Owner

It was the 8th issue that you closed! Thank you! Awesome! 👏

php-coder
php-coder reviewed Aug 7, 2019
View reviewed changes
src/main/resources/liquibase/version/0.4.1/2019-08-06--test_user_with_series_in_collection.xml
</insert>

<insert tableName="collections_series">
<column name="collection_id" valueComputed="(SELECT id FROM collections WHERE slug='seriesowner')" />
Copy link
Owner

Choose a reason for hiding this comment

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

There was a tiny formatting issue that I fixed in 76e636e Ideally, it's better to follow a similar approach in formatting everywhere, especially within a single file.

Copy link
Owner

Choose a reason for hiding this comment

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

@mukeshk ^^^ JFYI

@php-coder
Copy link
Owner

P.S. By the way, don't forget to remove merged branches -- https://github.com/php-coder/mystamps/wiki/after_merge_steps#remove-merged-branch

@mukeshk mukeshk deleted the gh1097_search_by_catalog_in_user_collection branch August 8, 2019 07:38
@php-coder php-coder mentioned this pull request Aug 16, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@php-coder php-coder php-coder left review comments

Assignees
No one assigned
Labels
resolution/merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mukeshk @mystamps-bot @php-coder