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

[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

[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

[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

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

[Corey Farrell]

> On Oct. 29, 2014, 5:18 a.m., wdoekes wrote:
> > /asterisk/trunk/tests/pbx/dialplan_reload/run-test, line 64
> > 
> >
> > 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
> > 
> >
> > 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

[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


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


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


"Restarted #%d" % self.count



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


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


"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

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

[Corey Farrell]

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

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