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

Issues reported for version 3.2.3 in Redmine that were not fixed in version 3.3.3 #547

Closed
enov opened this issue Sep 17, 2014 · 24 comments
Closed
Milestone

Comments

@enov
Copy link
Contributor

enov commented Sep 17, 2014

This is a placeholder issue for all Issues reported for version 3.2.3 in Redmine that were not fixed in version 3.3.3.

A new issue will be created and linked here, for each item below, with its title and description from Redmine.

The letter R is prefixed to the Redmine's issue number to avoid confusion between the issues of Github.

If you want to take an issue, drop me a line here, so that I assign it to you.

Hopefully we get through this and have a good 3.3.3 release :)


- [x] [R4791](http://dev.kohanaframework.org/issues/4791) Request_Client_Curl adds extra content-type header to all requests - [x] [R4778](http://dev.kohanaframework.org/issues/4778) PHPDoc comments - license location wrong - [x] [R4678](http://dev.kohanaframework.org/issues/4678) send_file() error on mime type - [x] [R4651](http://dev.kohanaframework.org/issues/4651) Request::uri() throws exception instead of returning a valid uri (docs obsolete) - [x] [R4627](http://dev.kohanaframework.org/issues/4627) shutdown_handler always uses 'Kohana_Exception::handler' instead of what's set in exception_handler() - [x] [R4625](http://dev.kohanaframework.org/issues/4625) Miscellaneous spelling errors - [x] [R4622](http://dev.kohanaframework.org/issues/4622) StdOut and StdErr Log Writers invalid log line format causes them not to output levels - [x] [R4613](http://dev.kohanaframework.org/issues/4613) Request class converts . to _ in url query string - [x] [R4607](http://dev.kohanaframework.org/issues/4607) Innacurate secure connection detection behind a load balancer - [x] [R4592](http://dev.kohanaframework.org/issues/4592) Response::send_file broken when no mime type specified - [x] [R4587](http://dev.kohanaframework.org/issues/4587) Request::redirect calls URL::site using the wrong parameters - [x] [R4568](http://dev.kohanaframework.org/issues/4568) DebugTest::test_dump fails because file has crlf line endings - [x] [R4522](http://dev.kohanaframework.org/issues/4522) Response does not respect Request's protocol - [x] [R4482](http://dev.kohanaframework.org/issues/4482) Array to string conversion in Arr::merge - [x] [R4287](http://dev.kohanaframework.org/issues/4287) Validation memory leak - [x] [R4241](http://dev.kohanaframework.org/issues/4241) It Must return original url with external link in request - [x] [R4235](http://dev.kohanaframework.org/issues/4235) Request::process_uri() must ignore external routes - [x] [R4201](http://dev.kohanaframework.org/issues/4201) Validation: field lable value - [x] [R4145](http://dev.kohanaframework.org/issues/4145) Translated messages should not rely on the default language being English - [x] [R4108](http://dev.kohanaframework.org/issues/4108) i18n __ function ignore $lang parameter - [x] [R4101](http://dev.kohanaframework.org/issues/4101) Cache, memcache, lifetime - [x] [R4079](http://dev.kohanaframework.org/issues/4079) Route::uri() should encode parameters - [x] [R3967](http://dev.kohanaframework.org/issues/3967) Kohana_RequestTest::test_url fails and Kohana_RequestTest::test_uri_only_trimed_on_internal fail for some custom routes - [x] [R3941](http://dev.kohanaframework.org/issues/3941) PHPDoc error for IntelliJ IDEA - [x] [R3896](http://dev.kohanaframework.org/issues/3896) Error message for 'password confirm' is incorrect in Auth module - [x] [R3788](http://dev.kohanaframework.org/issues/3788) Kohana 3.1 User model e-mail validation - [x] [R3604](http://dev.kohanaframework.org/issues/3604) Kohana_Session_Native should optionally Cookie Parameters when calling session_set_cookie_params - [x] [R3373](http://dev.kohanaframework.org/issues/3373) Kohana profiler Division by zero ErrorReflection [Warning] - [x] [R3931](http://dev.kohanaframework.org/issues/3931) Kohana_Exception::handler() doesn't exit with proper error codes - [x] [R4794](http://dev.kohanaframework.org/issues/4794) In online documentation, Error Handling page is throwing errors :) - [x] [R4275](http://dev.kohanaframework.org/issues/4275) kohanaframework.org/3.2 Guide Error - [x] [R4820](http://dev.kohanaframework.org/issues/4820) doc: incorrect mention of the missing method "Request::instance()" - [x] [R4814](http://dev.kohanaframework.org/issues/4814) Duplicate line of code wants to be removed - [x] [R4813](http://dev.kohanaframework.org/issues/4813) Kohana_UTF8Test::test_clean with data set number 3 array("\xFF", '') does not pass - [x] [R4706](http://dev.kohanaframework.org/issues/4706) Infinite loop in some of the assertion methods - [x] [R4322](http://dev.kohanaframework.org/issues/4322) More irregular pluralizations - [x] [R4092](http://dev.kohanaframework.org/issues/4092) Sub-requests don't inherit some properties properly from the parent - [x] [R3985](http://dev.kohanaframework.org/issues/3985) Kohana 3.1 User model e-mail validation - [x] [R3962](http://dev.kohanaframework.org/issues/3962) URL::query() should be consistent - [x] [R3946](http://dev.kohanaframework.org/issues/3946) Illegal offset type when call ArrayObject->offsetExists(arguments) - [x] [R3849](http://dev.kohanaframework.org/issues/3849) Failed 3 tests in Kohana_NumTest::test_format - [x] [R3716](http://dev.kohanaframework.org/issues/3716) Enforce uppercase constants and comparison operators - [x] [R3577](http://dev.kohanaframework.org/issues/3577) system/view/kohana/error.php should not be dependent on other classes - [x] [R4819](http://dev.kohanaframework.org/issues/4819) Replace preg_replace with /e modifier with preg_replace_callback - [x] [R4818](http://dev.kohanaframework.org/issues/4818) Add PHP 5.5 to Travis CI configuration - [x] [R4817](http://dev.kohanaframework.org/issues/4817) Add PHP 5.4 to Travis CI configuration - [x] [R4717](http://dev.kohanaframework.org/issues/4717) Why OAuth removed - [x] [R4676](http://dev.kohanaframework.org/issues/4676) Security::token() - [x] [R4675](http://dev.kohanaframework.org/issues/4675) Security::token() - [x] [R4623](http://dev.kohanaframework.org/issues/4623) Remove phpunit 3.6 and lower support - [x] [R4088](http://dev.kohanaframework.org/issues/4088) htaccess FastCgi - [x] [R4022](http://dev.kohanaframework.org/issues/4022) Arr::pluck() should use array_key_exists() - [x] [R4021](http://dev.kohanaframework.org/issues/4021) Arr::extract() should use array_key_exists() - [x] [R3973](http://dev.kohanaframework.org/issues/3973) Kohana_Cache_File Driver errors - [x] [R3925](http://dev.kohanaframework.org/issues/3925) Cookie::delete should uses Cookie::set - [x] [R3512](http://dev.kohanaframework.org/issues/3512) Conflicting standards dealing with whitespace - [x] [R3049](http://dev.kohanaframework.org/issues/3049) delete_all() - Error with 'file' driver - [x] [R3047](http://dev.kohanaframework.org/issues/3047) Cache error on __construct() - Linux machine - [x] [R4640](http://dev.kohanaframework.org/issues/4640) Template documentation broken and obsolete - [x] [R4624](http://dev.kohanaframework.org/issues/4624) Unsalted Cookies or Ingredients Docs - [x] [R4615](http://dev.kohanaframework.org/issues/4615) Arr::get should use both isset and array_key_exists - [x] [R4605](http://dev.kohanaframework.org/issues/4605) Unable to pass body params in external PUT requests (Windows) - [x] [R4601](http://dev.kohanaframework.org/issues/4601) Kohana::$magic_quotes always true on PHP 5.3.X
@enov enov added this to the 3.3.3 milestone Sep 17, 2014
@acoulton
Copy link
Member

@enov thanks for this. There's a couple that I fixed already but Redmine was down at the time so I couldn't comment and I don't have access to close.

  • R4817 (Add PHP5.4 to Travis) R4818 (Add PHP5.5 to Travis) are done - configured and passing for kohana/kohana and will be included as standard when I set up Travis for the other modules.
  • R4819 (preg_replace /e) is fixed, backported from 3.4 in 6b519fb
  • R4623 remove PHPUnit 3.6 support - is done AFAIK and closed on Redmine, we vendor 3.7 with composer.

I think there's a few others that show as closed on Redmine already - eg R3973 - I've not done a complete crosscheck.

I can try and take a couple next week, I have a few bits to get on with for the rest of this week.

@acoulton
Copy link
Member

@enov while working on the build/test setup am seeing there's quite a few places where unit tests are skipped - in particular CookieTest per kohana/kohana#47 and a few others.

Should we (I) try to get them working again in the 3.3 series? It's likely to require a bit of refactoring of the classes themselves (as well as the tests) to make them more testable and the tests more useful, but I think that can be done without changing anything about the public API.

@enov
Copy link
Contributor Author

enov commented Sep 17, 2014

Should we (I) try to get them working again in the 3.3 series? It's likely to require a bit of refactoring of the classes themselves (as well as the tests) to make them more testable and the tests more useful, but I think that can be done without changing anything about the public API.

I am not sure about this @acoulton . I looked around and I did not find a Kohana module or a OSS project in Kohana universe that extended Kohana_Cookie, except maybe just to define the cookie salt within the class. But still, I find it dangerous and I would say it should not be done for 3.3 if it requires refactoring.

I ticked a dozen issues above. These are the ones you mentioned, plus the ones that are already fixed or marked wontfix|worksforme.

Kindly, if someone can review the ones I ticked, before I move them to the bottom of the list and start working on the ones that need fixing. Just make sure that these do not require any fixing.

@acoulton
Copy link
Member

@enov I've reviewed your ticked issues and I agree that they're all valid to close.

Regarding:

I find it dangerous and I would say it should not be done for 3.3 if it requires refactoring.

Just to be clear, I'm talking tiny amounts of internal refactoring. For example for Kohana_Cookie just extracting the two setcookie calls to something like:

class Kohana_Cookie {
    public static function delete() {
        //return setcookie($name, NULL, -86400, Cookie::$path, Cookie::$domain, Cookie::$secure, Cookie::$httponly);
        return static::setcookie($name, NULL, -86400, Cookie::$path, Cookie::$domain, Cookie::$secure, Cookie::$httponly);
    }

    protected static function setcookie($name, $value, $expire, $path, $domain, $secure, $httponly)
    {
        return setcookie($name, $value, $expire, $path, $domain, $secure, $httponly);
    }
}

I can't really see any way that could affect anyone even if they've extended and wrapped/replaced the existing methods? I guess only if they've added a static setcookie method of their own for some reason - we could pick a more obscure name like static::_kohana_internal_mockable_setcookie to really minimise the risk.

That would allow us to extend the class for unit testing and mock out the setcookie call so that it's possible to test those methods after headers have been sent. Then I'd rewrite/expand the tests so that they cover some of the actual expected behaviour - for eg at the moment the Cookie::set tests basically just assert that setcookie returns true, they don't seem to cover any of the expiration, salting, whatever.

I think this is less dangerous than having these tests permanently skipped and will help ensure that any more major refactoring into 3.4 doesn't break anything that worked in 3.3.

@enov
Copy link
Contributor Author

enov commented Sep 18, 2014

I think this is less dangerous than having these tests permanently skipped and will help ensure that any more major refactoring into 3.4 doesn't break anything that worked in 3.3.

This is a valid point to make an exception. _kohana_internal_mockable_setcookie is ugly, I'd go for setcookie with the hope we don't break anything :) But we should be abusive with exceptions though ;)

@enov
Copy link
Contributor Author

enov commented Sep 18, 2014

@acoulton Except for the first two issues that you merged, I have ticked 9 items that do not require any action from us. Could you kindly review them?

acoulton added a commit that referenced this issue Sep 18, 2014
The cookie tests were all being skipped because headers are always sent
by the PHPUnit test runner (fixes kohana/kohana#47).

Additionally even when they ran they were covering very little of the actual
logic of the class, mostly just asserting that setcookie returns TRUE. Rewrote
the tests completely and slightly refactored the Cookie class to allow
mocking of the timestamp and setcookie calls.

Refs #547.
@acoulton
Copy link
Member

@enov:

  • R4820 Yes
  • R4814 Yes
  • R4813 Yes
  • R4706 Yes
  • R4322 Yes
  • R4092 Yes - this is for the future (if we keep HMVC).
  • R3985 Yes
  • R3962 Yes - this isn't doable in 3.3
  • R3946 Yes - invalid input is invalid input
  • R3849 Yes - this is a user build environment config issue
  • R3716 Yes - this is covered by the standard and I don't think we need to trawl for any last ones at this point.
  • R3577 Yes

Thanks, those all look good.

@enov
Copy link
Contributor Author

enov commented Sep 19, 2014

In order not to loose track, I have reopened the bugs R4092 and R3962 in Redmine, with status Feedback and rescheduled them for 3.4, as they are still valid bugs.

acoulton added a commit that referenced this issue Sep 22, 2014
The cookie tests were all being skipped because headers are always sent
by the PHPUnit test runner (fixes kohana/kohana#47).

Additionally even when they ran they were covering very little of the actual
logic of the class, mostly just asserting that setcookie returns TRUE. Rewrote
the tests completely and slightly refactored the Cookie class to allow
mocking of the timestamp and setcookie calls.

Refs #547.
acoulton added a commit that referenced this issue Sep 23, 2014
The cookie tests were all being skipped because headers are always sent
by the PHPUnit test runner (fixes kohana/kohana#47).

Additionally even when they ran they were covering very little of the actual
logic of the class, mostly just asserting that setcookie returns TRUE. Rewrote
the tests completely and slightly refactored the Cookie class to allow
mocking of the timestamp and setcookie calls.

Refs #547.
acoulton added a commit that referenced this issue Sep 23, 2014
The cookie tests were all being skipped because headers are always sent
by the PHPUnit test runner (fixes kohana/kohana#47).

Additionally even when they ran they were covering very little of the actual
logic of the class, mostly just asserting that setcookie returns TRUE. Rewrote
the tests completely and slightly refactored the Cookie class to allow
mocking of the timestamp and setcookie calls.

Refs #547.
@enov
Copy link
Contributor Author

enov commented Nov 11, 2014

@acoulton I have reviewed and ticked the remaining issues that are on the upper part of the list. There are few that I am not sure, I left them unchecked. Could you kindly review the checked ones?

@acoulton
Copy link
Member

@enov will try and work through this list tomorrow

@enov
Copy link
Contributor Author

enov commented Nov 19, 2014

Where art thou, @acoulton?

@acoulton
Copy link
Member

@enov sorry things have been crazy.

Have looked over them just now and these all seem fine to me:

  • R4791 Request_Client_Curl adds extra content-type header to all requests
  • R4778 PHPDoc comments - license location wrong
  • R4678 send_file() error on mime type
  • R4651 Request::uri() throws exception instead of returning a valid uri (docs obsolete)
  • R4627 shutdown_handler always uses 'Kohana_Exception::handler' instead of what's set in exception_handler()
  • R4625 Miscellaneous spelling errors
  • R4622 StdOut and StdErr Log Writers invalid log line format causes them not to output levels
  • R4613 Request class converts . to _ in url query string
  • R4607 Innacurate secure connection detection behind a load balancer
  • R4592 Response::send_file broken when no mime type specified
  • R4587 Request::redirect calls URL::site using the wrong parameters
  • R4568 DebugTest::test_dump fails because file has crlf line endings
  • R4522 Response does not respect Request's protocol
  • R4482 Array to string conversion in Arr::merge
  • R4287 Validation memory leak
  • R4241 It Must return original url with external link in request
  • R4235 Request::process_uri() must ignore external routes
  • R4201 Validation: field lable value
  • R4145 Translated messages should not rely on the default language being English
  • R4108 i18n __ function ignore $lang parameter
  • R4101 Cache, memcache, lifetime
  • R4079 Route::uri() should encode parameters
  • R3967 Kohana_RequestTest::test_url fails and Kohana_RequestTest::test_uri_only_trimed_on_internal fail for some custom routes
  • R3941 PHPDoc error for IntelliJ IDEA
  • R3896 Error message for 'password confirm' is incorrect in Auth module
  • R3788 Kohana 3.1 User model e-mail validation
  • R3604 Kohana_Session_Native should optionally Cookie Parameters when calling session_set_cookie_params
  • R3373 Kohana profiler Division by zero ErrorReflection [Warning]

Quite a few were either worksforme, wontfix, or had fixes that were already merged to 3.3 (sometimes 3.2) and I couldn't see anything outstanding.

@enov
Copy link
Contributor Author

enov commented Nov 19, 2014

We missed to see you around. I will take those issues you checked down in the list.

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

@acoulton

I have checked the remaining issues:

R3860 and R3859 both rely on R3552 which was set to Unscheduled in Redmine.

Please review whenever you have time.

@acoulton
Copy link
Member

@enov these look good to me, I think the two empty_rules issues should be closed and fixed in 3.4 with the R3552 solution or similar.

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

Created a new issue #575 so that we do not forget about it.

@acoulton
Copy link
Member

Ace. Possibly late in the day to suggest, but should we consider just running a one-time export from Redmine to Github - then we could be sure that nothing gets lost and stop using R... links? Someone must have built a tool for that, no?

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

Good idea @acoulton, but don't know of any tool for that.

@acoulton
Copy link
Member

@enov there's this - http://blogs.law.harvard.edu/rprasad/2014/07/10/moving-from-redmine-to-github-issues/ - I could try it out (on a dummy repo) and see what happens...

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

In Github, remaining issues to release 3.3.3:

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

Would you like me to add those above the big list on the top of this issue? Or should we differ those for a later release (3.3.4)?

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

Ah, there is also kohana/cache#50. I think, since all modules follow one versioning, we need to add the same Github milestones on all repos.

@acoulton
Copy link
Member

@enov I think those other github issues should go in 3.3.4 - they need a bit more work and aren't critical.

There's also a few open issues on Redmine that I didn't see on your list above? I think some may be invalid though. http://dev.kohanaframework.org/projects/kohana3/issues?query_id=66

@enov
Copy link
Contributor Author

enov commented Nov 26, 2014

Alright, will create a new issue as those are not reported for version 3.2.3.

Do you concur with me that we should close this one? 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants