Fix issue #84 #93
Fix issue #84 #93
Conversation
|
No clue how that build fails, i ran the tests twice and literally its impossible to fail given the change I made... |
|
I have played with it in awhile, but I think the issue is with the versions on travis CI. Their env has changed since I last made changes. |
|
Are you going to look into fixing the tests for the current TravisCI env? Since its quite difficult to contribute otherwise not knowing what failed because of env and what is legit broken, or I can send you screenshots of passed tests before pull requests but... |
|
To be honest, I don't really have any time to maintain this project at the moment, I wish I did. That being said, when you fork this repo, you should have a travis CI build available to you (or atleast you should be able to set one up with no issue), as there is a conf file in the project that tells travis how to work. |
|
I will try to resolve, the issues are because of ES version is the latest "1.1.1" on travis, when I change it to 1.1.1 for the tests it fails with the same errors. |
|
Fixed it to add support for elasticsearch 1.1.1 and updated all the dependencies too. It seems now that the build is passing. You can have a brief look over the test changes if you want and merge it. Actually I was thinking that removing the vendors from the repo makes sense, I dont see a reason to track them? |
|
I took a short glance and not sure that the changes are valid. It is important that any changes made are backwards compatible (which is why there are multiple version when tests are run). If features break between versions, the plugin needs to account for it (by either disabling the functionality, warning the user, etc). The biggest thing is I noticed you were changing the order of some of the test values, and many of the tests are written to specifically test the order in which results are returned. |
|
Yes, I tried to change the order of values only for tests where there isn't any specific weight to it, or ones that do not test sorting. And actually those are the ones that fail on new versions of ES, since if you put two values sequentially you are not guaranteed to get them in the same order if you query for them with no criteria (something that the prev. tests assumed). For tests where the order matters I tried to base the order on the date added (for example pagination) and not the the order of adding to the index (which in new ES is irrelevant when querying). I dont see any other way to fix those except either disregarding the order or changing all the search queries for those tests to have valid post dates and then query the ES by date to get a meaningful order, otherwise new ES will return random results on every fresh test run. Also the oldest recommended version of ES is in the 0.90.x branch so i changed the version.sh to account for that at this point 0.20 is beyond legacy and im not sure if there is reason to support it, as it will only introduce headaches. |
|
Gotcha, I will review more thoroughly. Did the tests fail on the old 0.20 version you replaced in the bash script? Also, big thank you btw for getting this green again! |
|
No problem i hope its fine now, but if you see anything suspicious let me know for sure and i will address it. And right now in 0.20 im failing only 1 test the Indexed::testClear(); I cant seem to find the reason for that yet though. Generally the clear index case has been problematic overall. |
|
i've merged this into 3.0, thanks! |
Simple fix for issue #84 and add PHPStorm files to .gitignore