[DB MIGRATION REQ'D] Implement and Enable DB-Backed Sessions #1818

Closed
wants to merge 33 commits into
from

Conversation

Projects
None yet
2 participants
@cdmoyer
Contributor

cdmoyer commented Feb 12, 2014

I think just close the other request. Needed to merge in your fix and some new fixes.

Creates SessionDAO Interface and MySQL implementation
Creates session table
Adds SessionCache::init() to start sessions and set handlers
Modifies controllers to make use of this
Moved date_default_timezone_set($config->getValue('timezone')); before SessionCache::init call.
The DateTime construct error was happening because in ThinkUpController, SessionCache::init was being called in __construct, before `initializeAppis called ingo``. The default timezone is set in``initializeApp``.

Fixes for failing tests from Session Changes
Handle new tables in installer test
Handle session already startin in other tests in Controller test.
Fix wrong table count in mysql install dao test.

Fixes for tests with database sessions.
Set a TU_MODE in setmode, check for that cookie
in the various places needed.

References #962

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 13, 2014

Member

I'm looking into the Travis build problem, but I'm also still getting WebTest failures. I see setmode uses a cookie now, but don't we need to change config.sample.inc.php to check for the cookie in this section?


//TESTS OVERRIDE: Assign variables below to use different settings during test builds
if ((isset($_SESSION["MODE"]) && $_SESSION["MODE"] == "TESTS") && ! isset($_SESSION["RD_MODE"])
|| (getenv("MODE")=="TESTS" && ! getenv("RD_MODE")=="1")) {
    //    $THINKUP_CFG['source_root_path']          = '/your-server-path-to/thinkup/';
    //    $THINKUP_CFG['db_user']                   = 'your_test_database_username';
    //    $THINKUP_CFG['db_password']               = 'your_test_database_password';
    //    $THINKUP_CFG['db_name']                   = 'your_test_database_name'; //by default, thinkup_tests
    $THINKUP_CFG['cache_pages']               = false;
    $THINKUP_CFG['debug']                     = true;
    $THINKUP_CFG['timezone']                  = 'UTC';
    ini_set('error_reporting', E_STRICT);
}

Member

ginatrapani commented Feb 13, 2014

I'm looking into the Travis build problem, but I'm also still getting WebTest failures. I see setmode uses a cookie now, but don't we need to change config.sample.inc.php to check for the cookie in this section?


//TESTS OVERRIDE: Assign variables below to use different settings during test builds
if ((isset($_SESSION["MODE"]) && $_SESSION["MODE"] == "TESTS") && ! isset($_SESSION["RD_MODE"])
|| (getenv("MODE")=="TESTS" && ! getenv("RD_MODE")=="1")) {
    //    $THINKUP_CFG['source_root_path']          = '/your-server-path-to/thinkup/';
    //    $THINKUP_CFG['db_user']                   = 'your_test_database_username';
    //    $THINKUP_CFG['db_password']               = 'your_test_database_password';
    //    $THINKUP_CFG['db_name']                   = 'your_test_database_name'; //by default, thinkup_tests
    $THINKUP_CFG['cache_pages']               = false;
    $THINKUP_CFG['debug']                     = true;
    $THINKUP_CFG['timezone']                  = 'UTC';
    ini_set('error_reporting', E_STRICT);
}

@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 13, 2014

Contributor

Ah!

That explains it. I just changed my config.inc.php and didn't change the
sample and commit it. I thought I was going crazy.

On Thu, Feb 13, 2014 at 9:23 AM, Gina Trapani notifications@github.comwrote:

I'm looking into the Travis build problem, but I'm also still getting
WebTest failures. I see setmode uses a cookie now, but don't we need to
change config.sample.inc.php to check for the cookie in this section?

//TESTS OVERRIDE: Assign variables below to use different settings during test builds
if ((isset($_SESSION["MODE"]) && $_SESSION["MODE"] == "TESTS") && ! isset($_SESSION["RD_MODE"])
|| (getenv("MODE")=="TESTS" && ! getenv("RD_MODE")=="1")) {
// $THINKUP_CFG['source_root_path'] = '/your-server-path-to/thinkup/';
// $THINKUP_CFG['db_user'] = 'your_test_database_username';
// $THINKUP_CFG['db_password'] = 'your_test_database_password';
// $THINKUP_CFG['db_name'] = 'your_test_database_name'; //by default, thinkup_tests
$THINKUP_CFG['cache_pages'] = false;
$THINKUP_CFG['debug'] = true;
$THINKUP_CFG['timezone'] = 'UTC';
ini_set('error_reporting', E_STRICT);
}

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-34982066
.

Contributor

cdmoyer commented Feb 13, 2014

Ah!

That explains it. I just changed my config.inc.php and didn't change the
sample and commit it. I thought I was going crazy.

On Thu, Feb 13, 2014 at 9:23 AM, Gina Trapani notifications@github.comwrote:

I'm looking into the Travis build problem, but I'm also still getting
WebTest failures. I see setmode uses a cookie now, but don't we need to
change config.sample.inc.php to check for the cookie in this section?

//TESTS OVERRIDE: Assign variables below to use different settings during test builds
if ((isset($_SESSION["MODE"]) && $_SESSION["MODE"] == "TESTS") && ! isset($_SESSION["RD_MODE"])
|| (getenv("MODE")=="TESTS" && ! getenv("RD_MODE")=="1")) {
// $THINKUP_CFG['source_root_path'] = '/your-server-path-to/thinkup/';
// $THINKUP_CFG['db_user'] = 'your_test_database_username';
// $THINKUP_CFG['db_password'] = 'your_test_database_password';
// $THINKUP_CFG['db_name'] = 'your_test_database_name'; //by default, thinkup_tests
$THINKUP_CFG['cache_pages'] = false;
$THINKUP_CFG['debug'] = true;
$THINKUP_CFG['timezone'] = 'UTC';
ini_set('error_reporting', E_STRICT);
}

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-34982066
.

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 13, 2014

Member

Yeah, looking at .travis.yml I think this config issue is related to the build problem. Travis uses the prefab config file in extras/dev/config and that's still referring to $_SESSION, too. I'll play around and see what I can do.

Member

ginatrapani commented Feb 13, 2014

Yeah, looking at .travis.yml I think this config issue is related to the build problem. Travis uses the prefab config file in extras/dev/config and that's still referring to $_SESSION, too. I'll play around and see what I can do.

@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 13, 2014

Contributor

I updated the sample and the extras config. Still seeing the build as failed on travis even though there are no errors. I feel like I'm in some sort of crazy rabbit hole. ;)

Contributor

cdmoyer commented Feb 13, 2014

I updated the sample and the extras config. Still seeing the build as failed on travis even though there are no errors. I feel like I'm in some sort of crazy rabbit hole. ;)

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 13, 2014

Member

Yeah, this stinks, because it's clearly a Travis-specific build problem. Okay, so here's the tedious-but-effective way I usually debug these things: First, remove all but one PHP version on .travis.yml to speed up the build, then comment things out in all_unit_tests and push to narrow down the problem. Is the problem in all_model_tests, all_controller_tests, or all_plugin_tests? Once you figure that out, start commenting out tests within those files to narrow it down further. SOMEthing somewhere is causing a the 255 return, we just have to see what it is.

Member

ginatrapani commented Feb 13, 2014

Yeah, this stinks, because it's clearly a Travis-specific build problem. Okay, so here's the tedious-but-effective way I usually debug these things: First, remove all but one PHP version on .travis.yml to speed up the build, then comment things out in all_unit_tests and push to narrow down the problem. Is the problem in all_model_tests, all_controller_tests, or all_plugin_tests? Once you figure that out, start commenting out tests within those files to narrow it down further. SOMEthing somewhere is causing a the 255 return, we just have to see what it is.

@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 13, 2014

Contributor

Phew.

So. For some reason session_start() and session_id() and friends are weird on Travis.
Also, SimpleTest seems to return 255 when there's an error, even when you call expectError(). Yeuch.

So. That table will grow pretty quickly, at first. PHP wants you to let it randomly clean up throughout the day, but in most high-load situations, people will set session.gc_probability to 0 and run a script during off-hours that cleans up the sessions table rather than letting the gc() method run.

Contributor

cdmoyer commented Feb 13, 2014

Phew.

So. For some reason session_start() and session_id() and friends are weird on Travis.
Also, SimpleTest seems to return 255 when there's an error, even when you call expectError(). Yeuch.

So. That table will grow pretty quickly, at first. PHP wants you to let it randomly clean up throughout the day, but in most high-load situations, people will set session.gc_probability to 0 and run a script during off-hours that cleans up the sessions table rather than letting the gc() method run.

tests/TestOfTestController.php
+ //$this->assertEqual('', session_id());
+
+
+ // simpletest returns 255 even if we expect these. Gah.

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 14, 2014

Member

So weird. Curious: why was SimpleTest not returning a 255 before? These expectations have been there for awhile.

@ginatrapani

ginatrapani Feb 14, 2014

Member

So weird. Curious: why was SimpleTest not returning a 255 before? These expectations have been there for awhile.

tests/TestOfTestController.php
+ //session_destroy(); // Make sure there's not session from previous tests.
+ //}
+ //$this->assertEqual('', session_id());
+ //$controller = new TestController(true);

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 14, 2014

Member

Is there a way to rewrite this test with the new session handling without using session_id, etc? We just want to assert the session doesn't get started when true is sent to the constructor and vice versa.

@ginatrapani

ginatrapani Feb 14, 2014

Member

Is there a way to rewrite this test with the new session handling without using session_id, etc? We just want to assert the session doesn't get started when true is sent to the constructor and vice versa.

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 14, 2014

Contributor

Yeah, I was pondering that. I'll play with it some more today.

On Fri, Feb 14, 2014 at 9:53 AM, Gina Trapani notifications@github.comwrote:

In tests/TestOfTestController.php:

@@ -187,4 +187,29 @@ public function testExceptionHandling() {
$this->assertEqual('Exception', $v_mgr->getTemplateDataItem('error_type'));
unset($_GET['text']);
}
+

  • /**
  • \* Test that a session is started when the controller has run
    
  • */
    
  • public function testSessionStarted() {
  •    // Fails in travis.  Hmm.
    
  •    //if (session_id() != '') {
    
  •        //session_destroy(); // Make sure there's not session from previous tests.
    
  •    //}
    
  •    //$this->assertEqual('', session_id());
    
  •    //$controller = new TestController(true);
    

Is there a way to rewrite this test with the new session handling without
using session_id, etc? We just want to assert the session doesn't get
started when true is sent to the constructor and vice versa.

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818/files#r9750208
.

@cdmoyer

cdmoyer Feb 14, 2014

Contributor

Yeah, I was pondering that. I'll play with it some more today.

On Fri, Feb 14, 2014 at 9:53 AM, Gina Trapani notifications@github.comwrote:

In tests/TestOfTestController.php:

@@ -187,4 +187,29 @@ public function testExceptionHandling() {
$this->assertEqual('Exception', $v_mgr->getTemplateDataItem('error_type'));
unset($_GET['text']);
}
+

  • /**
  • \* Test that a session is started when the controller has run
    
  • */
    
  • public function testSessionStarted() {
  •    // Fails in travis.  Hmm.
    
  •    //if (session_id() != '') {
    
  •        //session_destroy(); // Make sure there's not session from previous tests.
    
  •    //}
    
  •    //$this->assertEqual('', session_id());
    
  •    //$controller = new TestController(true);
    

Is there a way to rewrite this test with the new session handling without
using session_id, etc? We just want to assert the session doesn't get
started when true is sent to the constructor and vice versa.

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818/files#r9750208
.

@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 17, 2014

Contributor

Ok. I think maybe I have this working.
Funny thing about making all the tests pass, I found an actual issue with the DAO. ;)

Contributor

cdmoyer commented Feb 17, 2014

Ok. I think maybe I have this working.
Funny thing about making all the tests pass, I found an actual issue with the DAO. ;)

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 17, 2014

Member

Yes! I love it when tests actually catch bugs.
:-)

Sent from my mobile
On Feb 17, 2014 4:17 PM, "Chris Moyer" notifications@github.com wrote:

Ok. I think maybe I have this working.

Funny thing about making all the tests pass, I found an actual issue with
the DAO. ;)

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35323680
.

Member

ginatrapani commented Feb 17, 2014

Yes! I love it when tests actually catch bugs.
:-)

Sent from my mobile
On Feb 17, 2014 4:17 PM, "Chris Moyer" notifications@github.com wrote:

Ok. I think maybe I have this working.

Funny thing about making all the tests pass, I found an actual issue with
the DAO. ;)

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35323680
.

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 19, 2014

Member

Hey Chris - we are so close on this one. The last failures I'm getting are in one of the web tests that Travis does not run:

WebTestOfUpgradeDatabase.php
1) Text [ThinkUp's database needs an upgrade] not detected in [String: ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of upgrading. Please try back again in a little while. If you are the administrator of this ThinkUp installation, check your emai...] at [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testMigrations
    in WebTestOfUpgradeDatabase
Exception 1!
Unexpected exception of type [PDOException] with message [SQLSTATE[42S02]: Base table or view not found: 1146 Table 'thinkup_tests.tu_options' doesn't exist] in [/thinkup/tests/WebTestOfUpgradeDatabase.php line 406]
    in testMigrations
    in WebTestOfUpgradeDatabase
2) Text [ThinkUp's database needs an upgrade] not detected in [String: ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of upgrading. Please try back again in a little while. If you are the administrator of this ThinkUp installation, check your emai...] at [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
3) Text [ThinkUp's database needs an upgrade] not detected in [String: ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of upgrading. Please try back again in a little while. If you are the administrator of this ThinkUp installation, check your emai...] at [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
FAILURES!!!
Test cases run: 1/3, Passes: 195, Failures: 3, Exceptions: 1

This test is passing in the master branch, so something that got changed here isn't working in the database upgrades. That test is a particularly hairy one; take a shot at troubleshooting the failures and ping me if you get stuck.

Member

ginatrapani commented Feb 19, 2014

Hey Chris - we are so close on this one. The last failures I'm getting are in one of the web tests that Travis does not run:

WebTestOfUpgradeDatabase.php
1) Text [ThinkUp's database needs an upgrade] not detected in [String: ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of upgrading. Please try back again in a little while. If you are the administrator of this ThinkUp installation, check your emai...] at [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testMigrations
    in WebTestOfUpgradeDatabase
Exception 1!
Unexpected exception of type [PDOException] with message [SQLSTATE[42S02]: Base table or view not found: 1146 Table 'thinkup_tests.tu_options' doesn't exist] in [/thinkup/tests/WebTestOfUpgradeDatabase.php line 406]
    in testMigrations
    in WebTestOfUpgradeDatabase
2) Text [ThinkUp's database needs an upgrade] not detected in [String: ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of upgrading. Please try back again in a little while. If you are the administrator of this ThinkUp installation, check your emai...] at [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
3) Text [ThinkUp's database needs an upgrade] not detected in [String: ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of upgrading. Please try back again in a little while. If you are the administrator of this ThinkUp installation, check your emai...] at [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
FAILURES!!!
Test cases run: 1/3, Passes: 195, Failures: 3, Exceptions: 1

This test is passing in the master branch, so something that got changed here isn't working in the database upgrades. That test is a particularly hairy one; take a shot at troubleshooting the failures and ping me if you get stuck.

@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 19, 2014

Contributor

Ah. My shame is exposed, that I have that test commented out because I get
errors even on master.

1) Text [Success! Your account has been activated. Please log in.] not
detected in [String: ( ! ) Strict standards: Declaration of
SmartyThinkUp::assign() should be compatible with Smarty::assign($tpl_var,
$value = NULL) in
/Users/cdmoyer/Sites/ThinkUp/webapp/test_installer/thinkup/_lib/mode...] at
[/Users/cdmoyer/Sites/ThinkUp/tests/WebTestOfUpgradeDatabase.php line 241]

in testMigrations

in WebTestOfUpgradeDatabase

I'll investigate both.

Contributor

cdmoyer commented Feb 19, 2014

Ah. My shame is exposed, that I have that test commented out because I get
errors even on master.

1) Text [Success! Your account has been activated. Please log in.] not
detected in [String: ( ! ) Strict standards: Declaration of
SmartyThinkUp::assign() should be compatible with Smarty::assign($tpl_var,
$value = NULL) in
/Users/cdmoyer/Sites/ThinkUp/webapp/test_installer/thinkup/_lib/mode...] at
[/Users/cdmoyer/Sites/ThinkUp/tests/WebTestOfUpgradeDatabase.php line 241]

in testMigrations

in WebTestOfUpgradeDatabase

I'll investigate both.

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 19, 2014

Member

Two for the price of one! TEST_DEBUG is a huge help figuring out what's
going wrong with this test (which I've spent a good amount of time doing
over the years).

On Wed, Feb 19, 2014 at 2:00 PM, Chris Moyer notifications@github.comwrote:

Ah. My shame is exposed, that I have that test commented out because I get
errors even on master.


1) Text [Success! Your account has been activated. Please log in.] not
detected in [String: ( ! ) Strict standards: Declaration of
SmartyThinkUp::assign() should be compatible with Smarty::assign($tpl_var,
$value = NULL) in
/Users/cdmoyer/Sites/ThinkUp/webapp/test_installer/thinkup/_lib/mode...]
at
[/Users/cdmoyer/Sites/ThinkUp/tests/WebTestOfUpgradeDatabase.php line 241]

in testMigrations

in WebTestOfUpgradeDatabase

I'll investigate both.

On Wed, Feb 19, 2014 at 12:59 PM, Gina Trapani notifications@github.comwrote:

Hey Chris - we are so close on this one. The last failures I'm getting
are
in one of the web tests that Travis does not run:

WebTestOfUpgradeDatabase.php

  1. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testMigrations
    in WebTestOfUpgradeDatabase
    Exception 1!
    Unexpected exception of type [PDOException] with message
    [SQLSTATE[42S02]: Base table or view not found: 1146 Table
    'thinkup_tests.tu_options' doesn't exist] in
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 406]
    in testMigrations
    in WebTestOfUpgradeDatabase
  2. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
  3. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
    FAILURES!!!
    Test cases run: 1/3, Passes: 195, Failures: 3, Exceptions: 1

This test is passing in the master branch, so something that got changed
here isn't working in the database upgrades. That test is a particularly
hairy one; take a shot at troubleshooting the failures and ping me if
you
get stuck.

Reply to this email directly or view it on GitHub<
https://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35527884>
.

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35534696
.

http://ginatrapani.org

Member

ginatrapani commented Feb 19, 2014

Two for the price of one! TEST_DEBUG is a huge help figuring out what's
going wrong with this test (which I've spent a good amount of time doing
over the years).

On Wed, Feb 19, 2014 at 2:00 PM, Chris Moyer notifications@github.comwrote:

Ah. My shame is exposed, that I have that test commented out because I get
errors even on master.


1) Text [Success! Your account has been activated. Please log in.] not
detected in [String: ( ! ) Strict standards: Declaration of
SmartyThinkUp::assign() should be compatible with Smarty::assign($tpl_var,
$value = NULL) in
/Users/cdmoyer/Sites/ThinkUp/webapp/test_installer/thinkup/_lib/mode...]
at
[/Users/cdmoyer/Sites/ThinkUp/tests/WebTestOfUpgradeDatabase.php line 241]

in testMigrations

in WebTestOfUpgradeDatabase

I'll investigate both.

On Wed, Feb 19, 2014 at 12:59 PM, Gina Trapani notifications@github.comwrote:

Hey Chris - we are so close on this one. The last failures I'm getting
are
in one of the web tests that Travis does not run:

WebTestOfUpgradeDatabase.php

  1. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testMigrations
    in WebTestOfUpgradeDatabase
    Exception 1!
    Unexpected exception of type [PDOException] with message
    [SQLSTATE[42S02]: Base table or view not found: 1146 Table
    'thinkup_tests.tu_options' doesn't exist] in
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 406]
    in testMigrations
    in WebTestOfUpgradeDatabase
  2. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
  3. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
    FAILURES!!!
    Test cases run: 1/3, Passes: 195, Failures: 3, Exceptions: 1

This test is passing in the master branch, so something that got changed
here isn't working in the database upgrades. That test is a particularly
hairy one; take a shot at troubleshooting the failures and ping me if
you
get stuck.

Reply to this email directly or view it on GitHub<
https://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35527884>
.

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35534696
.

http://ginatrapani.org

@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 19, 2014

Contributor

Whoa. Does this normally take over an hour?

On Feb 19, 2014, at 2:28 PM, Gina Trapani notifications@github.com wrote:

Two for the price of one! TEST_DEBUG is a huge help figuring out what's
going wrong with this test (which I've spent a good amount of time doing
over the years).

On Wed, Feb 19, 2014 at 2:00 PM, Chris Moyer notifications@github.comwrote:

Ah. My shame is exposed, that I have that test commented out because I get
errors even on master.


1) Text [Success! Your account has been activated. Please log in.] not
detected in [String: ( ! ) Strict standards: Declaration of
SmartyThinkUp::assign() should be compatible with Smarty::assign($tpl_var,
$value = NULL) in
/Users/cdmoyer/Sites/ThinkUp/webapp/test_installer/thinkup/_lib/mode...]
at
[/Users/cdmoyer/Sites/ThinkUp/tests/WebTestOfUpgradeDatabase.php line 241]

in testMigrations

in WebTestOfUpgradeDatabase

I'll investigate both.

On Wed, Feb 19, 2014 at 12:59 PM, Gina Trapani notifications@github.comwrote:

Hey Chris - we are so close on this one. The last failures I'm getting
are
in one of the web tests that Travis does not run:

WebTestOfUpgradeDatabase.php

  1. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testMigrations
    in WebTestOfUpgradeDatabase
    Exception 1!
    Unexpected exception of type [PDOException] with message
    [SQLSTATE[42S02]: Base table or view not found: 1146 Table
    'thinkup_tests.tu_options' doesn't exist] in
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 406]
    in testMigrations
    in WebTestOfUpgradeDatabase
  2. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
  3. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
    FAILURES!!!
    Test cases run: 1/3, Passes: 195, Failures: 3, Exceptions: 1

This test is passing in the master branch, so something that got changed
here isn't working in the database upgrades. That test is a particularly
hairy one; take a shot at troubleshooting the failures and ping me if
you
get stuck.

Reply to this email directly or view it on GitHub<
https://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35527884>
.

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35534696
.

http://ginatrapani.org

Reply to this email directly or view it on GitHub.

Contributor

cdmoyer commented Feb 19, 2014

Whoa. Does this normally take over an hour?

On Feb 19, 2014, at 2:28 PM, Gina Trapani notifications@github.com wrote:

Two for the price of one! TEST_DEBUG is a huge help figuring out what's
going wrong with this test (which I've spent a good amount of time doing
over the years).

On Wed, Feb 19, 2014 at 2:00 PM, Chris Moyer notifications@github.comwrote:

Ah. My shame is exposed, that I have that test commented out because I get
errors even on master.


1) Text [Success! Your account has been activated. Please log in.] not
detected in [String: ( ! ) Strict standards: Declaration of
SmartyThinkUp::assign() should be compatible with Smarty::assign($tpl_var,
$value = NULL) in
/Users/cdmoyer/Sites/ThinkUp/webapp/test_installer/thinkup/_lib/mode...]
at
[/Users/cdmoyer/Sites/ThinkUp/tests/WebTestOfUpgradeDatabase.php line 241]

in testMigrations

in WebTestOfUpgradeDatabase

I'll investigate both.

On Wed, Feb 19, 2014 at 12:59 PM, Gina Trapani notifications@github.comwrote:

Hey Chris - we are so close on this one. The last failures I'm getting
are
in one of the web tests that Travis does not run:

WebTestOfUpgradeDatabase.php

  1. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testMigrations
    in WebTestOfUpgradeDatabase
    Exception 1!
    Unexpected exception of type [PDOException] with message
    [SQLSTATE[42S02]: Base table or view not found: 1146 Table
    'thinkup_tests.tu_options' doesn't exist] in
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 406]
    in testMigrations
    in WebTestOfUpgradeDatabase
  2. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
  3. Text [ThinkUp's database needs an upgrade] not detected in [String:
    ThinkUp Get ThinkUp Log In ThinkUp ThinkUp is currently in the process of
    upgrading. Please try back again in a little while. If you are the
    administrator of this ThinkUp installation, check your emai...] at
    [/thinkup/tests/WebTestOfUpgradeDatabase.php line 350]
    in testFailAndRerunMigration
    in WebTestOfUpgradeDatabase
    FAILURES!!!
    Test cases run: 1/3, Passes: 195, Failures: 3, Exceptions: 1

This test is passing in the master branch, so something that got changed
here isn't working in the database upgrades. That test is a particularly
hairy one; take a shot at troubleshooting the failures and ping me if
you
get stuck.

Reply to this email directly or view it on GitHub<
https://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35527884>
.

Reply to this email directly or view it on GitHubhttps://github.com/ginatrapani/ThinkUp/pull/1818#issuecomment-35534696
.

http://ginatrapani.org

Reply to this email directly or view it on GitHub.

@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 20, 2014

Member

The first time you run it it downloads every release of ThinkUp ever published (41 releases, about 53MB) and that can be really slow. Once they're all downloaded into your build folder, it doesn't take that long. Timing it here, it finishes in 3 minutes.

Member

ginatrapani commented Feb 20, 2014

The first time you run it it downloads every release of ThinkUp ever published (41 releases, about 53MB) and that can be really slow. Once they're all downloaded into your build folder, it doesn't take that long. Timing it here, it finishes in 3 minutes.

cdmoyer added some commits Feb 5, 2014

[DB MIGRATION REQ'D] Implement and Enable DB-Backed Sessions
Creates SessionDAO Interface and MySQL implementation
Creates session table
Adds SessionCache::init() to start sessions and set handlers
Modifies controllers to make use of this
Moved date_default_timezone_set($config->getValue('timezone')); before SessionCache::init call.
   The DateTime construct error was happening because in ThinkUpController, SessionCache::init was being called in ``__construct``, before ``initializeApp``` is called in ``go``. The default timezone is set in ``initializeApp``.

Fixes for failing tests from Session Changes
  Handle new tables in installer test
  Handle session already startin in other tests in Controller test.
  Fix wrong table count in mysql install dao test.

Fixes for tests with database sessions.
  Set a TU_MODE in setmode, check for that cookie
  in the various places needed.
destroy session at end of controller test.
Because otherwise the register_shutdown_function handler tries to do
so, and the tables have already been cleaned up.
@cdmoyer

This comment has been minimized.

Show comment Hide comment
@cdmoyer

cdmoyer Feb 24, 2014

Contributor

Yikes.

Ok, running all the tests. I gave up and spun up an ec2 large instance, which runs these in minutes. As opposed to my laptop which was taking an hour just for the one test everytime.

So, in order to make all the tests pass, I needed to add a config option to enable database-backed sessions, for when the migration tests don't have the sessions table. Or get it mid-way through and suddenly the test user is logged out.

I think this is probably a good advanced config option, anyways, because file-based sessions might be better in some situations, anyways.

[ec2-user@ip-10-43-155-96 ThinkUp]$ git status
# On branch 962-Move-sessions-to-db-take2
nothing to commit, working directory clean
[ec2-user@ip-10-43-155-96 ThinkUp]$ php tests/all_tests.php 
Model tests

OK
Test cases run: 64/64, Passes: 5206, Failures: 0, Exceptions: 0
Plugin tests
OK
Test cases run: 65/65, Passes: 2695, Failures: 0, Exceptions: 0
Integration tests
OK
Test cases run: 11/11, Passes: 196, Failures: 0, Exceptions: 0
Installer tests
OK
Test cases run: 2/2, Passes: 4657, Failures: 0, Exceptions: 0
Controller tests
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
OK
Test cases run: 41/41, Passes: 2715, Failures: 0, Exceptions: 0

Total ThinkUp test passes: 15469
Total ThinkUp test failures: 0
Time elapsed: 14 minute(s)

52675 total words of application documentation

[ec2-user@ip-10-43-155-96 ThinkUp]$ echo $?
0
Contributor

cdmoyer commented Feb 24, 2014

Yikes.

Ok, running all the tests. I gave up and spun up an ec2 large instance, which runs these in minutes. As opposed to my laptop which was taking an hour just for the one test everytime.

So, in order to make all the tests pass, I needed to add a config option to enable database-backed sessions, for when the migration tests don't have the sessions table. Or get it mid-way through and suddenly the test user is logged out.

I think this is probably a good advanced config option, anyways, because file-based sessions might be better in some situations, anyways.

[ec2-user@ip-10-43-155-96 ThinkUp]$ git status
# On branch 962-Move-sessions-to-db-take2
nothing to commit, working directory clean
[ec2-user@ip-10-43-155-96 ThinkUp]$ php tests/all_tests.php 
Model tests

OK
Test cases run: 64/64, Passes: 5206, Failures: 0, Exceptions: 0
Plugin tests
OK
Test cases run: 65/65, Passes: 2695, Failures: 0, Exceptions: 0
Integration tests
OK
Test cases run: 11/11, Passes: 196, Failures: 0, Exceptions: 0
Installer tests
OK
Test cases run: 2/2, Passes: 4657, Failures: 0, Exceptions: 0
Controller tests
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
rm: cannot remove `build/thinkup/data/.ht*': No such file or directory
OK
Test cases run: 41/41, Passes: 2715, Failures: 0, Exceptions: 0

Total ThinkUp test passes: 15469
Total ThinkUp test failures: 0
Time elapsed: 14 minute(s)

52675 total words of application documentation

[ec2-user@ip-10-43-155-96 ThinkUp]$ echo $?
0
@ginatrapani

This comment has been minimized.

Show comment Hide comment
@ginatrapani

ginatrapani Feb 24, 2014

Member

Wow! I'm impressed. And deeply, deeply sorry you had to go through all this to run the whole suite.

I will review this code soon - most likely tomorrow.

Now that you know our test suite as well as one can and have just gone through this whole rigamarole, how do you feel about taking a stab at #1328?

Member

ginatrapani commented Feb 24, 2014

Wow! I'm impressed. And deeply, deeply sorry you had to go through all this to run the whole suite.

I will review this code soon - most likely tomorrow.

Now that you know our test suite as well as one can and have just gone through this whole rigamarole, how do you feel about taking a stab at #1328?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment