Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload

2014-10-31 Thread Corey Farrell

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4122/
---

(Updated Oct. 31, 2014, 8:02 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and Scott Griepentrog.


Changes
---

Committed in revision 5832


Repository: testsuite


Description
---

In some configurations 3 seconds is not enough of a delay before Asterisk is 
fully booted, preventing core restart gracefully from succeeding.  This 
causes many iterations to be skipped, and in some cases the test never ends.

Make use of core waitfullybooted to delay restarts.  Remove unused global 
variables workingdir and testdir, add global variable restart_iterations to 
specify 50 runs.  Decrease reactor_timeout from 300 to 30, use reset_timeout 
instead.


Diffs
-

  /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 

Diff: https://reviewboard.asterisk.org/r/4122/diff/


Testing
---

Yes


Thanks,

Corey Farrell

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload

2014-10-30 Thread Scott Griepentrog

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4122/#review13631
---

Ship it!


The test originally needed the 50 iterations to be sure to catch the fault, but 
as it's just checking for a specific regression 5 will do fine.

- Scott Griepentrog


On Oct. 29, 2014, 6:27 a.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4122/
 ---
 
 (Updated Oct. 29, 2014, 6:27 a.m.)
 
 
 Review request for Asterisk Developers and Scott Griepentrog.
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 In some configurations 3 seconds is not enough of a delay before Asterisk is 
 fully booted, preventing core restart gracefully from succeeding.  This 
 causes many iterations to be skipped, and in some cases the test never ends.
 
 Make use of core waitfullybooted to delay restarts.  Remove unused global 
 variables workingdir and testdir, add global variable restart_iterations to 
 specify 50 runs.  Decrease reactor_timeout from 300 to 30, use reset_timeout 
 instead.
 
 
 Diffs
 -
 
   /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 
 
 Diff: https://reviewboard.asterisk.org/r/4122/diff/
 
 
 Testing
 ---
 
 Yes
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload

2014-10-29 Thread wdoekes

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4122/#review13604
---



/asterisk/trunk/tests/pbx/dialplan_reload/run-test
https://reviewboard.asterisk.org/r/4122/#comment24117

I don't quite see what happens here:

- the test does a 'core restart gracefully' 50 times?

Does that fit in 30 seconds now?

Or.. that is the reset_timeout() below? In that case a little comment 
somewhere would help, so I wouldn't be surprised when the test runs for longer 
than 30 seconds and succeeds.



/asterisk/trunk/tests/pbx/dialplan_reload/run-test
https://reviewboard.asterisk.org/r/4122/#comment24116

Is it needed to callLater() instead of just calling it? Applies to the 
other 0-second callLaters too. You'll defer handling soon enough.

The only other example where I find a callLater(0, in the testsuite, is 
broken anyway:

reactor.callLater(0, self.launch_test())  # 
tests/fastagi/record-file/run-test




/asterisk/trunk/tests/pbx/dialplan_reload/run-test
https://reviewboard.asterisk.org/r/4122/#comment24115

Restarted #%d % self.count



/asterisk/trunk/tests/pbx/dialplan_reload/run-test
https://reviewboard.asterisk.org/r/4122/#comment24113

Why do we wait (an arbitrary) 3 seconds for this? Can't we call the 
fullybooted_run immediately?



/asterisk/trunk/tests/pbx/dialplan_reload/run-test
https://reviewboard.asterisk.org/r/4122/#comment24114

Restarting #%d % self.count


Lastly: PEP says space after a comma, please.

- wdoekes


On Oct. 28, 2014, 10:20 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4122/
 ---
 
 (Updated Oct. 28, 2014, 10:20 p.m.)
 
 
 Review request for Asterisk Developers and Scott Griepentrog.
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 In some configurations 3 seconds is not enough of a delay before Asterisk is 
 fully booted, preventing core restart gracefully from succeeding.  This 
 causes many iterations to be skipped, and in some cases the test never ends.
 
 Make use of core waitfullybooted to delay restarts.  Remove unused global 
 variables workingdir and testdir, add global variable restart_iterations to 
 specify 50 runs.  Decrease reactor_timeout from 300 to 30, use reset_timeout 
 instead.
 
 
 Diffs
 -
 
   /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 
 
 Diff: https://reviewboard.asterisk.org/r/4122/diff/
 
 
 Testing
 ---
 
 Yes
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload

2014-10-29 Thread Corey Farrell


 On Oct. 29, 2014, 5:18 a.m., wdoekes wrote:
  /asterisk/trunk/tests/pbx/dialplan_reload/run-test, line 64
  https://reviewboard.asterisk.org/r/4122/diff/1/?file=68588#file68588line64
 
  Is it needed to callLater() instead of just calling it? Applies to the 
  other 0-second callLaters too. You'll defer handling soon enough.
  
  The only other example where I find a callLater(0, in the testsuite, 
  is broken anyway:
  
  reactor.callLater(0, self.launch_test())  # 
  tests/fastagi/record-file/run-test
 

Also removed other calls to reactor.callLater.


 On Oct. 29, 2014, 5:18 a.m., wdoekes wrote:
  /asterisk/trunk/tests/pbx/dialplan_reload/run-test, line 74
  https://reviewboard.asterisk.org/r/4122/diff/1/?file=68588#file68588line74
 
  Why do we wait (an arbitrary) 3 seconds for this? Can't we call the 
  fullybooted_run immediately?

I think it might be something to do with our running 'restart'.  At a certain 
point the CLI is not available, so fullybooted_run fails without the delay (or 
with 1 second delay).  Maybe it could be decreased to 2 seconds but that would 
make the test more sensitive.


On Oct. 29, 2014, 5:18 a.m., Corey Farrell wrote:
  Lastly: PEP says space after a comma, please.

All other results of pep8 in this file also fixed.


- Corey


---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4122/#review13604
---


On Oct. 28, 2014, 6:20 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4122/
 ---
 
 (Updated Oct. 28, 2014, 6:20 p.m.)
 
 
 Review request for Asterisk Developers and Scott Griepentrog.
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 In some configurations 3 seconds is not enough of a delay before Asterisk is 
 fully booted, preventing core restart gracefully from succeeding.  This 
 causes many iterations to be skipped, and in some cases the test never ends.
 
 Make use of core waitfullybooted to delay restarts.  Remove unused global 
 variables workingdir and testdir, add global variable restart_iterations to 
 specify 50 runs.  Decrease reactor_timeout from 300 to 30, use reset_timeout 
 instead.
 
 
 Diffs
 -
 
   /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 
 
 Diff: https://reviewboard.asterisk.org/r/4122/diff/
 
 
 Testing
 ---
 
 Yes
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Re: [asterisk-dev] [Code Review] 4122: testsuite: Fix freeze on tests/pbx/dialplan_reload

2014-10-29 Thread Corey Farrell

---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4122/
---

(Updated Oct. 29, 2014, 7:27 a.m.)


Review request for Asterisk Developers and Scott Griepentrog.


Repository: testsuite


Description
---

In some configurations 3 seconds is not enough of a delay before Asterisk is 
fully booted, preventing core restart gracefully from succeeding.  This 
causes many iterations to be skipped, and in some cases the test never ends.

Make use of core waitfullybooted to delay restarts.  Remove unused global 
variables workingdir and testdir, add global variable restart_iterations to 
specify 50 runs.  Decrease reactor_timeout from 300 to 30, use reset_timeout 
instead.


Diffs (updated)
-

  /asterisk/trunk/tests/pbx/dialplan_reload/run-test 5803 

Diff: https://reviewboard.asterisk.org/r/4122/diff/


Testing
---

Yes


Thanks,

Corey Farrell

-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev