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

Add tests for piwik/searchengine-and-social-list#14 #10394

Closed
wants to merge 0 commits into from

Conversation

piwikjapan
Copy link

Add tests for matomo-org/searchengine-and-social-list#14
Use kanji strings instead of just 'Piwik'.

@piwikjapan
Copy link
Author

piwikjapan commented Aug 13, 2016

Maybe, mb_detect_encoding which indicated multiple Japanese encodes did not work on this travis-ci environment.

@mattab
Copy link
Member

mattab commented Aug 16, 2016

Hi @piwikjapan
Thanks for the pull request!

the build is failing on Travis CI - do the tests work for you locally?

@mattab mattab added this to the 3.0.0 milestone Aug 16, 2016
@mattab mattab added Bug For errors / faults / flaws / inconsistencies etc. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. Needs Review PRs that need a code review labels Aug 16, 2016
@mattab
Copy link
Member

mattab commented Aug 16, 2016

@piwikjapan since the changes in matomo-org/searchengine-and-social-list#14 were merged only few days ago, to have those changes included in your pull request, you must use the version 1.3.1 of this library (try running composer update piwik/piwik/searchengine-and-social-list)

@piwikjapan
Copy link
Author

I updated to the version 1.3.1, and ./console tests:run ./plugins/Referrers/tests/Unit/SearchEngineTest.php shown below:

  1. Piwik\Plugins\Referrers\tests\SearchEngineTest::testMissingSearchEngineIcons with data set adding support for windows phone 7 version 7.5 #1 ('www.118700.se', array(array('q'), 'sok.aspx?q={k}', '118 700'))
    www.118700.se
    Failed asserting that false is true.

/home/yamachan/piwik/plugins/Referrers/tests/Unit/SearchEngineTest.php:144

  1. Piwik\Plugins\Referrers\tests\SearchEngineTest::testMissingSearchEngineIcons with data set Minor adminmenu improvements #22 ('www.allaverksamheter.se', array(array('What'), 'SearchResult.aspx?What={k}', 'Allaverksamheter'))
    www.allaverksamheter.se
    Failed asserting that false is true.

/home/yamachan/piwik/plugins/Referrers/tests/Unit/SearchEngineTest.php:144

  1. Piwik\Plugins\Referrers\tests\SearchEngineTest::testMissingSearchEngineIcons with data set Parse replay-to header in the same way like from #332 ('www.isodelen.se', array(array('Keywords'), 'sokresultat?Keywords={k}', 'Isodelen'))
    www.isodelen.se
    Failed asserting that false is true.

/home/yamachan/piwik/plugins/Referrers/tests/Unit/SearchEngineTest.php:144

  1. Piwik\Plugins\Referrers\tests\SearchEngineTest::testMissingSearchEngineIcons with data set explode(): Empty delimiter in archive.sh daily run #424 ('www.riksdelen.se', array(array('What'), 'SearchResult.aspx?What={k}', 'Riksdelen'))
    www.riksdelen.se
    Failed asserting that false is true.

but there are not essential and all my tests (that's I am asking to merge) were successful.

@sgiehl
Copy link
Member

sgiehl commented Aug 17, 2016

we need to add some icons for those new search engines. Btw. could you please undo the changes to the access mode of core/Tracker/PageUrl.php. You've changed it from 100644 → 100755

@piwikjapan
Copy link
Author

I watched https://travis-ci.org/piwik/piwik/jobs/151990012 carefully,

Piwik\Plugins\Referrers\tests\SearchEngineTest::testExtractInformationFromUrl with data set #75
(Successfully with utf-8. Therefore, it does not show anything.)

l. 1158
3) Piwik\Plugins\Referrers\tests\SearchEngineTest::testExtractInformationFromUrl with data set #76
Unsuccessfully with Shift_JIS.

Array (
'name' => 'Yahoo! Japan Images'

  • 'keywords' => '君の名は。'
  • 'keywords' => '?n?̖??́b'
    )

l. 1171
4) Piwik\Plugins\Referrers\tests\SearchEngineTest::testExtractInformationFromUrl with data set #77
Unsuccessfully with EUC-JP.
Array (
'name' => 'Yahoo! Japan Images'

  • 'keywords' => '君の名は。'
  • 'keywords' => '????̾?ϡ?'
    )

On Travis CI,
I thought that all tests except UTF-8 fell. But another tests except UTF-8 (e.g. with EUC-KR) has been successfully.
Therefore, the problem shown when some charsets in SearchEngines.yml, the first charset will be successful, but the second and third will fail.

It means,

actual condition:

Yahoo! Japan Images:

urls:
  - image.search.yahoo.co.jp
params:
  - p
backlink: 'search?p={k}'
charsets:
  - utf-8     -> success
  - euc-jp    -> fail
  - ms932     -> fail

my imagination:

charsets:
  - ms932     -> will be successful
  - utf-8     -> will fail
  - euc-jp    -> will fail

or:

charsets:
  - euc-jp    -> will be successful
  - ms932     -> will fail
  - utf-8     -> will fail

@sgiehl
Copy link
Member

sgiehl commented Aug 17, 2016

Travis won't use the new version of piwik/searchengine-and-social-list unless the composer.lock is changed here.

@mattab
Copy link
Member

mattab commented Aug 22, 2016

@piwikjapan try running composer update piwik/piwik/searchengine-and-social-list in your branch and push the updated composer.lock to the pull request, this may fix your issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants