<div>Hi</div><div><br></div><div><br></div><div><span style="color: rgb(160, 160, 168); ">On Saturday, 23 March 2013 at 8:24 PM, Marc Delisle wrote:</span></div>
<blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;">
<span><div>Le 2013-03-23 08:47, Ayush Chaudhary a écrit :</div><blockquote type="cite"><div>Hi,</div></blockquote><div><br></div><div>Hello,</div><div><br></div><blockquote type="cite"><div><div><br></div><div>I was going through the existing tests. The test 'testIsHttps' under</div><div>test/classes/PMA_Config_test.php looks wrong as isHttps() should be</div><div>returning true for the second test case. However, the function</div><div>isHttps() uses a local static variable, which is why the test is</div><div>passing as in the construct, the PmaAbsoluteUri is initialised with</div><div>null and isHttps() is hence returning false in all subsequent calls.</div><div><br></div><div>I am not sure why a local static variable was used here. </div></div></blockquote><div><br></div><div>To avoid retesting the condition for nothing?</div><div><br></div><blockquote type="cite"><div><div>A better</div><div>test for this method would be if we could reset the static variable</div><div>before each assertion. Since its a local static variable, I couldn't</div><div>find a proper way to reset the static variable. Should we not move</div><div>the static variable in the scope of the class? In fact, even if we</div><div>don't use a static variable, we could just make is_https a class</div><div>member and initialise it to NULL in the constructor. This would</div><div>eventually achieve the same result in a neater way.</div></div></blockquote><div><br></div><div>Makes sense, but watch out: isHttps() and detectHttps() are both using</div><div>$is_https, for different goals.</div><blockquote type="cite"><div><div><br></div><div>Also, what approach do we take for resetting static variables if</div><div>needed for unit testing? One thing I came across was using</div><div>Reflection_Helper::set_static_variable($class, $name, $new_value)?</div></div></blockquote></span></blockquote><blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;"><span><div><div><div></div><div>Yes, this approach seems interesting :</div><div><a href="http://tech.blog.box.com/2012/07/unit-testing-with-static-variables/">http://tech.blog.box.com/2012/07/unit-testing-with-static-variables/</a></div><div><br></div><div><br></div><div>-- </div><div>Marc Delisle</div><div><a href="http://infomarc.info">http://infomarc.info</a></div><div><br></div><div>------------------------------------------------------------------------------</div><div>Everyone hates slow websites. So do we.</div><div>Make your web apps faster with AppDynamics</div><div>Download AppDynamics Lite for free today:</div><div><a href="http://p.sf.net/sfu/appdyn_d2d_mar">http://p.sf.net/sfu/appdyn_d2d_mar</a></div><div>_______________________________________________</div><div>Phpmyadmin-devel mailing list</div><div><a href="mailto:Phpmyadmin-devel@lists.sourceforge.net">Phpmyadmin-devel@lists.sourceforge.net</a></div><div><a href="https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel">https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel</a></div></div></div></span>
</blockquote>
<div>
I think the purpose of the static variable is to only let isHttps() execute once in the lifecycle of a PMA_Config object. </div><div>Regarding, the use of $is_https in detectHttps(), the copy of $is_https in detectHttps() is completely independent of the one in isHttps() because isHttps() creates a <b>local </b>copy of the variable. So detectHttps() would not need to be changed at all. </div><div><br></div><div>Making $is_https a member variable and using the member variable in isHttps() will help to make tests more exhaustive as we will be able to test for </div><div>1. non-https input</div><div>2. https input</div><div>3. input when is_https is already set (this is the only case that is being tested right now)</div><div><br></div><div>Also, regarding that approach, the issue here is that the static copy has scope only within the function isHttps(), so it does not seem possible to reset it using Reflection_Helper. </div><div><br></div><div>Please let me know, if this would be fine. I can then write a quick patch to fix this, and also improve testIsHttps()</div><div><br></div><div>Thanks,</div><div>Ayush</div>