Jens-G opened a new pull request, #3907:
URL: https://github.com/apache/thrift/pull/3907
`lib/ts` was in no `SUBDIRS` block — only in `EXTRA_DIST` — so `make check`
never entered it, and
`lib/ts/Makefile.am`'s `check-local` never ran. That is the ticket. Wiring
it up turned out not
to be enough, because of a second problem underneath:
**`make check` reported success while running nothing.** `check-local` in
both `lib/js` and
`lib/ts` is guarded by `if HAVE_NPM`, and `configure.ac` called
`AC_PATH_PROG([NPM], [npm])` only
*inside* the `--with-nodejs` and `--with-nodets` blocks. Any build that
disables the nodejs
binding — which is what every per-binding CI job does — left `NPM` empty, so
the conditional was
false, `check-local` was not even defined, and:
```
$ make -C lib/ts check
make check-local
make[1]: Nothing to be done for 'check-local'.
$ echo $?
0
```
Two browser test suites have been reporting green without executing a single
assertion.
`AM_CONDITIONAL(HAVE_NPM, ...)` was also declared twice.
So this change has three parts:
- `configure.ac`: look for npm before the binding blocks and declare
`HAVE_NPM` once. Only
`lib/js/Makefile.am` and `lib/ts/Makefile.am` consume that conditional;
`lib/nodejs` and
`lib/nodets` use `$(NPM)` but are gated on `WITH_NODEJS` / `WITH_NODETS`,
which are untouched.
- `lib/Makefile.am`: `SUBDIRS += ts` inside the `WITH_JAVA` block, next to
`js` and for the same
reason — the TypeScript test drives the same ant target to fetch its
browser libraries.
`ts` stays in `EXTRA_DIST`, as `js` does, so it still ships when java is
absent.
- `.github/workflows/build.yml`: a `lib-ts` job, because no job runs a
top-level `make check` —
every one of them runs `make -C lib/<lang> check`, and neither `js` nor
`ts` was in that set.
The job carries two assertions, so that it cannot repeat the failure it
exists to prevent: the
configure summary must show the Java library enabled, and the check output
must contain a QUnit
count. Without the second one a run that tested nothing would still exit 0.
Verified locally in a container (Ubuntu 22.04, ant, Java 17, Node 22.23.2),
autotools rather than
a bare `npx grunt`:
| | before | after |
|---|---|---|
| `make -C lib/ts check` | "Nothing to be done", exit 0, 0 tests | exit 0,
4x lint free, 25 + 1 tests, 118 assertions, 0 failed |
| `make -C lib/js check` | "Nothing to be done", exit 0, 0 tests | exit 0,
every page green |
Getting there needed the four blockers that are now on master: THRIFT-6317
and THRIFT-6318 in the
generator, THRIFT-6321 for the lint levels, and THRIFT-6322 for the
symbol-keyed struct methods.
Each was found by refusing to accept the silent green above.
The workflow change validates as YAML and zizmor reports no findings.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
