<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 2:</p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 2: Code-Review-1</p><p style="white-space: pre-wrap; word-wrap: break-word;">(3 comments)</p><p style="white-space: pre-wrap; word-wrap: break-word;">The concern that comes to mind is the test becoming fragile in the future if we use taskprocessors in other ways, thus having to continue to update and tweak the test condition. Thoughts Richard and Kevin?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Anything that processes Asterisk CLI output is going to be fragile. The CLI output is not intended to be stable. It is targeted for human consumption. The output of "core show taskprocessors" has changed several times as new things are tracked. The task processor names have changed a few times too. The "outsess" task processor name series has changed from its original naming as internal organization changed.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">I agree it is somewhat fragile, but I think most, if not all the test conditions scrape the CLI. Also as long as the screen column order doesn't change (which I don't think it has in a while if at all) then that fragility is lessened except ....for the hard-coded named "outsess".</p><p style="white-space: pre-wrap; word-wrap: break-word;">Not really sure of a good way around this. It can be moved to a configurable list, but then that'd potentially have to change.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm fine allowing this for now though. Hopefully taskprocessors, and their names are fairly stabilized for now. If it becomes a problem we can reassess later.</p><p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/testsuite/+/14210">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/lib/python/asterisk/taskprocessor_test_condition.py">File lib/python/asterisk/taskprocessor_test_condition.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/lib/python/asterisk/taskprocessor_test_condition.py@67">Patch Set #2, Line 67:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">remove whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/lib/python/asterisk/taskprocessor_test_condition.py@72">Patch Set #2, Line 72:</a> <code style="font-family:monospace,monospace"> if task_processor.processor.startswith('pjsip/outsess/'):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">If possible I think I'd prefer this to not be hard-coded (here at least), but configurable through yaml, or at the very least make it a global variable specified near the top of the file for easy access/changing. As well I think making it a list of processor items to ignore would be good. And/Or perhaps even allow regex matching.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/lib/python/asterisk/taskprocessor_test_condition.py@74">Patch Set #2, Line 74:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespace to remove</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/lib/python/asterisk/taskprocessor_test_condition.py@77">Patch Set #2, Line 77:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">more whitespace here too</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/tests/apps/queues/redirect/configs/ast1/extensions.conf">File tests/apps/queues/redirect/configs/ast1/extensions.conf:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/14210/2/tests/apps/queues/redirect/configs/ast1/extensions.conf@9">Patch Set #2, Line 9:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Additional whitespace to remove</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/testsuite/+/14210">change 14210</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/testsuite/+/14210"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: testsuite </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I90451cca574e2fa7d825b4cdc0bab03e331c0e9b </div>
<div style="display:none"> Gerrit-Change-Number: 14210 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: lvl <digium@lvlconsultancy.nl> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 29 Apr 2020 18:14:56 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>