search series by catalog number and name #1086
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generated by 🚫 Danger |
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
mukeshk
force-pushed
the
gh340_search_series_by_catalog
branch
from
940f656 to
32c3c63
Compare
|
Please check |
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
php-coder
reviewed
Jul 17, 2019
|
P.S. could you also modify commit message a little bit? -test(series/search/validation.robot): finding series by catalog number
+test(series/search/validation.robot): add tests for searching the series by a catalog number. |
mukeshk
force-pushed
the
gh340_search_series_by_catalog
branch
from
32c3c63 to
dc7b7c0
Compare
|
php-coder
reviewed
Jul 18, 2019
php-coder
reviewed
Jul 18, 2019
|
This works for now. Is this good ?
Page Should Contain Element css=.search-results [href="/series/1"]
…On Thu, Jul 18, 2019 at 6:20 PM Slava Semushin ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/test/robotframework/series/search/logic.robot
<#1086 (comment)>:
> *** Keywords ***
Before Test Suite
Open Browser ${SITE_URL}/ ${BROWSER}
Register Keyword To Run On Failure Log Source
+
+Search Series By Catalog By Name And Number
+ [Arguments] ${catalogName} ${catalogNumber}
+ Go To ${SITE_URL}
+ Input Text id=catalogNumber ${catalogNumber}
+ Select From List By Value id=catalogName ${catalogName}
+ Submit Form id=search-series-form
+ Link Should Point To id=series-1 ${SITE_URL}/series/1
Why do we want the last link? It seems like the tests fail exactly because
of that:
http://127.0.0.1:8080/series/1 != http://127.0.0.1:8080/series/9
Hm.. Actually, I know why is that.
SQL queries that we use for searching
<https://github.com/php-coder/mystamps/blob/41db2ea697366937314cba069d30c8cc44757c38/src/main/resources/sql/stamps_catalog_dao_queries.properties#L181-L221>
don't have ORDER BY so the ordering depends on a database. H2 orders the
items from the last to the first, while MySQL orders opposite. That's why
integration tests fail in one case and pass in another (because different
databases were used).
What the order should be is a another question and it's out of scope of
this task. But our tests shouldn't depend on that order at all.
So, let's use a different approach (that even better actually) in this
case:
Page Should Contain Element css=.search-results [href="${SITE_URL}/series/1"]
See https://css-tricks.com/attribute-selectors/ for details about CSS
selectors.
@mukeshk <https://github.com/mukeshk> Let me know if this works :)
Perhaps, we need some concatenation for ${SITE_URL} here. In this case,
we can also use Concatenate keyword (see
src/test/robotframework/selenium.utils.robot for details).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1086?email_source=notifications&email_token=AAJPE6FEAKOTT2OT34KTRMTQABRLDA5CNFSM4IEPRJVKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB63LC6Y#discussion_r304898567>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJPE6EFO2K4QNFNPSVUR6DQABRLDANCNFSM4IEPRJVA>
.
|
|
-- this works -- space 99 issue comment. I am not clear |
So, the link doesn't have http://127.0.0.1:8080 part? Only /series/1 ? |
|
Yes, only /series/1 worked
…On Thu 18 Jul, 2019, 8:52 PM Slava Semushin, ***@***.***> wrote:
This works for now. Is this good ?
Page Should Contain Element css=.search-results [href="/series/1"]
So, the link doesn't have http://127.0.0.1:8080 part? Only /series/1 ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1086?email_source=notifications&email_token=AAJPE6B3Q5UVP3XDCQUZ6DDQACDEBA5CNFSM4IEPRJVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2I2YEA#issuecomment-512863248>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJPE6AYUVNDFHYAJDY5JSTQACDEBANCNFSM4IEPRJVA>
.
|
|
OK got the idea. Will commit changes in next 30 min.
…On Thu 18 Jul, 2019, 8:54 PM Slava Semushin, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/test/robotframework/series/search/logic.robot
<#1086 (comment)>:
> @@ -13,7 +13,23 @@ Search series by non-existing catalog number
Submit Form id=search-series-form
Element Text Should Be id=no-series-found No series found
+Search series by existing catalog number
+ [Template] Search Series By Catalog By Name And Number
+ michel 99
+ scott 99
+ yvert 99
+ gibbons 99
My idea was to align it like this:
Search series by existing catalog number
[Template] Search Series By Catalog By Name And Number
michel 99
scott 99
yvert 99
gibbons 99
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1086?email_source=notifications&email_token=AAJPE6G6IMBZZWQFOWWRJKDQACDKTA5CNFSM4IEPRJVKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB64EEEA#discussion_r304978644>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJPE6HQW44CHYBGATKIAWLQACDKTANCNFSM4IEPRJVA>
.
|
mukeshk
force-pushed
the
gh340_search_series_by_catalog
branch
from
dc7b7c0 to
3361096
Compare
|
Completed the changes suggested |
php-coder
requested changes
Jul 18, 2019
mukeshk
force-pushed
the
gh340_search_series_by_catalog
branch
2 times, most recently
from
0445781 to
1cb9e0f
Compare
php-coder
requested changes
Jul 18, 2019
…ies by a catalog number. Fixes php-coder#340
mukeshk
force-pushed
the
gh340_search_series_by_catalog
branch
from
1cb9e0f to
37ead1e
Compare
|
Can you check now? Thanks for reviewing. |
Heh, my fault: it should be |
|
@mukeshk Thank you! I've updated https://github.com/php-coder/mystamps/wiki/team-members accordingly. |
|
thanks :)
…On Fri, Jul 19, 2019 at 10:22 PM Slava Semushin ***@***.***> wrote:
@mukeshk <https://github.com/mukeshk> Thank you! I've updated
https://github.com/php-coder/mystamps/wiki/team-members accordingly.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1086?email_source=notifications&email_token=AAJPE6FHE4F5UC62OOLTGQ3QAHWLDA5CNFSM4IEPRJVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2MFKEQ#issuecomment-513299730>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJPE6GBRAZ6QCDVIBOBWL3QAHWLDANCNFSM4IEPRJVA>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addressed to #340