[
https://issues.apache.org/jira/browse/THRIFT-6113?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6113.
--------------------------------
Fix Version/s: 0.25.0
Resolution: Fixed
> test_keyword_escape.py regression test silently skips (thrift compiler not
> found) in CI and local builds
> --------------------------------------------------------------------------------------------------------
>
> Key: THRIFT-6113
> URL: https://issues.apache.org/jira/browse/THRIFT-6113
> Project: Thrift
> Issue Type: Bug
> Components: Python - Library
> Reporter: Jens Geyer
> Priority: Minor
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> The Python keyword-escaping regression test added in THRIFT-5927
> (lib/py/test/test_compiler/test_keyword_escape.py) never actually exercises
> the compiler in CI, and can pick up the wrong compiler locally.
> Root causes:
> 1. lib/py/Makefile.am's check-local target invokes the test as a plain
> "$(PYTHON) test/test_compiler/test_keyword_escape.py", without passing the
> THRIFT make variable through as an environment variable. Every other
> Makefile.am in the tree that needs the compiler at test time defines/exports
> THRIFT explicitly (e.g. "THRIFT = $(top_builddir)/compiler/cpp/thrift"); this
> one doesn't, so the test script's os.environ['THRIFT'] lookup is always empty
> when run via "make check".
> 2. test_keyword_escape.py's find_thrift() fallback then tries two
> relative-path guesses to locate the compiler in the build tree, but both are
> off by one directory level (3 x "../" instead of 4, from
> lib/py/test/test_compiler/), so they resolve to lib/compiler/cpp/thrift and
> lib/build/compiler/cpp/bin/thrift instead of the correct compiler/cpp/thrift
> and build/compiler/cpp/bin/thrift.
> With neither the environment variable nor the fallback working, and no
> system-wide "thrift" in PATH on a clean CI runner, find_thrift() returns None
> and the test prints "WARNING: thrift compiler not found, skipping test" and
> returns 0 (success). This has been true on every CI run since the THRIFT-5927
> fix was merged (see .github/workflows/build.yml, lib-python job, which
> downloads the compiler artifact to exactly compiler/cpp/thrift -- the path
> the broken fallback fails to reach), so the regression test currently
> provides no actual coverage in CI.
> Locally, the same missing-env-var issue means find_thrift() falls through to
> a PATH search ('thrift', /usr/local/bin/thrift, /usr/bin/thrift) before ever
> trying the build tree; if a different thrift binary happens to be on PATH
> (e.g. from a prior "make install" or a distro package), the test silently
> exercises that binary instead of the one just built, which can resurface
> already-fixed issues under a misleading error.
> Fix: pass THRIFT=$(THRIFT) through explicitly on the check-local recipe line,
> matching the convention used elsewhere in the tree, and correct the
> off-by-one in the relative-path fallback so standalone invocations (without
> make) still work too.
> Found while investigating a report on THRIFT-5927 that the fix appeared
> broken on master; confirmed the generator fix itself is correct -- this is a
> test/build-infrastructure gap only.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
