-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 test for emoji in comments #27332
Conversation
@davitol would be cool to also have a test with emoji in a file name, just in case. |
43664f2
to
493dd95
Compare
I am not getting this to work:
I've added changes from https://github.com/owncloud/core/blob/master/config/config.sample.php#L1084-L1107 in /etc/mysql/mariadb.conf.d/50-server.cnf using ubuntu 16 and maria 10.0. I've also tried using ubuntu 14.04 mysql server 5.5, also not working. Perhaps the steps are not clear enough to set the environment. |
Ok, the environment has worked changing the file my.cnf. Like it is recommended here https://www.drupal.org/node/2754539 . But when running the second test it fails and returns an exception: Scenario: Creating a comment with an emoji on a file name with an emoji belonging to myself # /var/www/html/master_08-03-17/tests/integration/features/comments.feature:32
[Mon Mar 13 14:17:54 2017] 127.0.0.1:36428 [404]: /ocs/v2.php/cloud/users/user0
[Mon Mar 13 14:17:55 2017] 127.0.0.1:36430 [200]: /ocs/v1.php/cloud/users
[Mon Mar 13 14:17:55 2017] 127.0.0.1:36432 [200]: /ocs/v1.php/cloud/users/user0
[Mon Mar 13 14:17:56 2017] 127.0.0.1:36434 [200]: /ocs/v2.php/cloud/users/user0
Given user "user0" exists # FeatureContext::assureUserExists()
Given As an "user0" # FeatureContext::asAn()
[Mon Mar 13 14:17:56 2017] 127.0.0.1:36436 [201]: /remote.php/webdav/myFileToComment.txt
Given User "user0" uploads file "data/😃.txt" to "/myFileToComment.txt" # FeatureContext::userUploadsAFileTo()
[Mon Mar 13 14:17:56 2017] 127.0.0.1:36438 [207]: /remote.php/webdav//myFileToComment.txt
[Mon Mar 13 14:17:56 2017] Exception: {"Message":"HTTP\/1.1 400 Invalid actor \"\"","Exception":"Sabre\\DAV\\Exception\\BadRequest","Code":0,"Trace":"#0 \/var\/www\/html\/master_08-03-17\/apps\/comments\/lib\/Dav\/CommentsPlugin.php(123): OCA\\Comments\\Dav\\CommentsPlugin->createComment('files', '413', NULL, 'application\/jso...')\n#1 [internal function]: OCA\\Comments\\Dav\\CommentsPlugin->httpPost(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#2 \/var\/www\/html\/master_08-03-17\/lib\/composer\/sabre\/event\/lib\/EventEmitterTrait.php(105): call_user_func_array(Array, Array)\n#3 \/var\/www\/html\/master_08-03-17\/lib\/composer\/sabre\/dav\/lib\/DAV\/Server.php(479): Sabre\\Event\\EventEmitter->emit('method:POST', Array)\n#4 \/var\/www\/html\/master_08-03-17\/lib\/composer\/sabre\/dav\/lib\/DAV\/Server.php(254): Sabre\\DAV\\Server->invokeMethod(Object(Sabre\\HTTP\\Request), Object(Sabre\\HTTP\\Response))\n#5 \/var\/www\/html\/master_08-03-17\/apps\/dav\/lib\/Server.php(227): Sabre\\DAV\\Server->exec()\n#6 \/var\/www\/html\/master_08-03-17\/apps\/dav\/appinfo\/v2\/remote.php(31): OCA\\DAV\\Server->exec()\n#7 \/var\/www\/html\/master_08-03-17\/remote.php(165): require_once('\/var\/www\/html\/m...')\n#8 {main}","File":"\/var\/www\/html\/master_08-03-17\/apps\/comments\/lib\/Dav\/CommentsPlugin.php","Line":240,"User":"user0"}
[Mon Mar 13 14:17:56 2017] 127.0.0.1:36440 [400]: /remote.php/dav/comments/files/413/
When "user0" posts a comment with content "�~_~X~\" on the file named "/myFileToComment.txt" it should return "201" # CommentsContext::postsACommentWithContentOnTheFileNamedItShouldReturn()
Response status code was not 201 (400) (Exception) |
Seems like this function is not ready to use filename with emojis: Warning: file_get_contents(http://localhost:8080/remote.php/webdav/😃.txt): failed to open stream: HTTP request failed! in /var/www/html/master_08-03-17/tests/integration/features/bootstrap/CommentsContext.php line 87 Also this function seems to have problems with filenames starting with '/': It is not very flexible, it is always using user0 in its fields hardcoded. Probably this comments context requires a refactor to be more integrated with the rest of the tests. |
@SergioBertolinSG I have removed the scenario of naming the file with emojis and I have included in the PR only the test of setting a comment with an emoji. Please retest and let's merge this if it works and I'll work in the other scenario in a separately PR |
Just clean the commits (I recommend to put as fixup all commits but first one) and it is 👍 . |
5d46e83
to
8e1b691
Compare
Rebase to trigger jenkins again, seems a problem with it. |
8e1b691
to
2629bf8
Compare
@PVince81 this needs changes in jenkins before merge I guess. Preparing mysql to use 4 byte chars. |
450bf99
to
8bf809c
Compare
8bf809c
to
673b18b
Compare
any update ? since then I think we already have a runner for mb4 ? |
@davitol please revive/redo after checking whether we already have such tests before. note that we might need to skip said test when running on an old non-mb4 compatible database |
@davitol look in https://github.com/owncloud/core/blob/master/tests/acceptance/features/apiComments/createComments.feature - that has a scenario with an emoji in a comment. |
@phil-davis Agree to spread the test with those two scenarios. Feel free to write them if you want to. |
See issue #34419 - I assigned myself. |
Add testcase 'make a comment with an emoji'
@SergioBertolinSG Please review