brbzull0 opened a new issue, #13617:
URL: https://github.com/apache/trafficserver/issues/13617
### Impact
Test-only defect, no production impact. Introduced by #13146, now on
`master`.
Test G of `config_reload_plugin_api.test.py` is the only test asserting that
a **core**
reload task does not carry the `[plugin: <name>]` attribution tag that
`traffic_ctl config status` adds for plugin-owned tasks.
`ExcludesExpression` compiles its
first argument as a regex, and the unescaped brackets in `'ip_allow
[plugin]'` make it a
character class rather than a literal. The pattern cannot match the
command's output in
the correct case *or* in the regressed case, so the assertion passes
unconditionally.
If core reload tasks ever started being reported as plugin-owned, this test
would still go
green and the regression would ship undetected. Nothing else covers that
behaviour.
```
Version: master @ 83335d4eff9694061409d889c73627a847f46d24
Platform: test-only; not platform specific
Config: none
```
### Proof
`ExcludesExpression` compiles its first argument as a regex and applies
`re.search` per
line, and `IncludesExpression` is an alias of `ContainsExpression`, so all
three matchers
behave this way:
```python
# autest/testers/__init__.py:16
from .contains_expression import ContainsExpression as IncludesExpression
# autest/testers/excludes_expression.py:38-42
if isinstance(regexp, str):
regexp = re.compile(regexp, reflags)
```
The assertion, at
https://github.com/apache/trafficserver/blob/83335d4eff9694061409d889c73627a847f46d24/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py#L271-L272
```python
tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression(
'ip_allow [plugin]', 'Core task ip_allow must not have [plugin] tag')
```
`[plugin]` is a character class matching one of `p l u g i n`, so the
pattern is
`ip_allow ` followed by one of those six characters. The tag it is meant to
detect is
built at
https://github.com/apache/trafficserver/blob/83335d4eff9694061409d889c73627a847f46d24/src/traffic_ctl/CtrlPrinters.cc#L345-L350
```cpp
// Build label and right-aligned duration
std::string label = std::string(status_icon(f.status)) + " " + fname;
if (!f.meta.plugin_name.empty()) {
label += " [plugin: " + f.meta.plugin_name + "]";
}
```
so the character following `ip_allow ` is `[`, which is not in the class.
Found by
inspection, then confirmed by running the exact pattern the test passes:
```python
import re
pat = re.compile('ip_allow [plugin]') # exactly what the test passes
good = " * ip_allow ................ 12ms" # core task, no tag
bad = " * ip_allow [plugin: my_plugin] .... 12ms" # the regression it
guards
for line in (good, bad):
print(bool(pat.search(line)))
```
Observed:
```
core task (want: no match) search=False ExcludesExpression -> PASS
wrongly tagged (want: MATCH) search=False ExcludesExpression -> PASS
```
Expected, the second case must fail. With `r'ip_allow.*\[plugin:'`:
```
core task (want: no match) search=False ExcludesExpression -> PASS
wrongly tagged (want: MATCH) search=True ExcludesExpression -> FAIL
```
The two positive assertions in the same file are already written correctly as
`r'\[plugin: '` (lines 130 and 167); this negative one was missed.
A sweep of all 502 files under `tests/gold_tests` on `master` found 12 other
bracketed
matchers, all of them the intentional `[Uu]sing HTTP/?2` in the `h2` tests.
This is the
only instance of the defect.
### Proposed change
```diff
--- a/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py
+++ b/tests/gold_tests/jsonrpc/config_reload_plugin_api.test.py
@@ -268,8 +268,8 @@
tr.Processes.Default.Env = ts.Env
tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression(
- 'ip_allow [plugin]', 'Core task ip_allow must not have [plugin] tag')
+tr.Processes.Default.Streams.stdout = Testers.ExcludesExpression(
+ r'ip_allow.*\[plugin:', 'Core task ip_allow must not have a [plugin:
<name>] tag')
tr.StillRunningAfter = ts
```
`.*` rather than a literal space, because the label goes through
`sanitize_label()` and a
dot fill sits between the task name and the duration, so the tag is not
adjacent to the
name at every column width. Matching the `[plugin:` prefix rather than a full
`[plugin: <name>]` keeps the assertion independent of the plugin name.
No production code changes, no compatibility impact, nothing to backport
beyond wherever
#13146 lands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
