You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "glaterza (via GitHub)" <gi...@apache.org> on 2026/10/13 16:57:19 UTC
[PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
glaterza opened a new pull request, #45247:
URL: https://github.com/apache/superset/pull/45247
### SUMMARY
Fixes #45245.
#44395 added two tests that run the real msgcat step of `scripts/translations/babel_update.sh`. They never run on CI, and one of them fails on any machine that has gettext.
- **They skip on CI.** Python-Unit does not install gettext, so `shutil.which("msgcat")` is `None` and both tests skip. The JUnit report of the Python-Unit run on master at `7aadf35e7d` lists both as skipped (https://github.com/apache/superset/actions/runs/38381761342).
- **One fails where gettext exists.** The test runs the script with a restricted `PATH` (`/usr/bin:/bin` plus stubs). The script calls `python` on line 77. macOS, Ubuntu 26.04 and Debian 13 have no `/usr/bin/python`. The trailing-line test fails with `line 77: python: command not found`.
- **The failing-msgcat test passed for the wrong reason.** With the msgcat `|| exit 1` removed, the script still stopped before `pybabel update`. It stopped one line later, at the missing `python`.
This PR changes three things:
1. Both end-to-end tests put `python` and `python3` shims on the restricted `PATH`. Each shim runs the interpreter that runs pytest. A shared `_stage_script` helper replaces the setup that two tests repeated.
2. Python-Unit and Python-Nightly-Canary install gettext. The step copies the one in `superset-translations.yml`, with the same pinned action. Like that step, it leaves `execute_install_scripts` off. The gettext package needs no post-install script.
3. The two tests call `_require_msgcat()`. It skips when msgcat is missing. It fails instead when `SUPERSET_GETTEXT_STRICT` is set. Both workflows set that variable at job level.
Item 3 follows the `SUPERSET_TESTCONTAINERS_STRICT` pattern from #43502 (`tests/testcontainers/db_engine_specs/_driver.py`). I did not key it on `CI`. GitHub, GitLab, CircleCI and Buildkite all set `CI`, so a pipeline outside this repository without gettext would get two new failures. As with `SUPERSET_TESTCONTAINERS_STRICT`, any non-empty value turns strict mode on.
`babel_update.sh` itself does not change.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Local runs at `256e6cb7f1`, on master `139a06eee1`.
Before (master's test file, macOS with Homebrew gettext):
```
FAILED babel_update_test.py::test_the_trailing_line_strip_keeps_the_last_entry_intact
babel_update.sh: line 77: python: command not found
1 failed, 5 passed
```
After (this branch):
```
6 passed
```
The same runs in Docker, with only `python3`, `python3-pytest` and `gettext` from apt:
| Image | `/usr/bin/python` | Master's test file | This branch |
|---|---|---|---|
| `ubuntu:26.04` (the CI runner's release) | none | 1 failed, 5 passed | 6 passed |
| `ubuntu:26.04` with `python-is-python3` | present | not run | 6 passed |
| `debian:13` | none | 1 failed, 5 passed | 6 passed |
Each test fails when the behavior it guards breaks. Each test stays green on a harmless change:
| Change to a copy of `babel_update.sh` or to the test | Test | Master | This branch |
|---|---|---|---|
| Remove `\|\| exit 1` after msgcat | failing-msgcat test | passes | fails |
| Change it to `\|\| exit 2` | failing-msgcat test | passes | passes |
| Remove the `python` shims (macOS) | trailing-line test | n/a | fails |
| Change the script's `python` call to `python3` | trailing-line test | passes | passes |
Without msgcat on `PATH`:
```
SUPERSET_GETTEXT_STRICT=true: 2 failed ("SUPERSET_GETTEXT_STRICT is set, but gettext's msgcat is not on PATH: install gettext in this job"), 4 passed
CI=true only: 2 skipped ("gettext's msgcat is not installed on this machine"), 4 passed
nothing set: 2 skipped, 4 passed
```
Python-Nightly-Canary on my fork, with Python 3.13.15, on the first commit of this branch. The gettext step has not changed since.
```
Install gettext tools 14:45:57 → 14:46:04 (7 s)
tests/unit_tests/scripts/translations/babel_update_test.py ...... [ 73%]
tests/unit_tests/websocket/test_channel.py ............. [100%]
##[error]The operation was canceled.
```
All six tests passed, and none skipped. The job then hit its 45-minute timeout, 3 seconds after the last test. The canary on master has the same timeout problem, which #45246 describes.
### TESTING INSTRUCTIONS
1. Install gettext (`brew install gettext` or `apt-get install gettext`).
2. Run `pytest tests/unit_tests/scripts/translations/babel_update_test.py`. Expect 6 passed. On master, expect the trailing-line test to fail with `python: command not found`.
3. Remove gettext from `PATH` and run the same command with `SUPERSET_GETTEXT_STRICT=true`. Expect 2 failed. Without the variable, expect 2 skipped.
4. In this PR's Python-Unit run, open the `junit-results-current` artifact. It lists both msgcat tests as passed, not skipped.
All 136 tests in `tests/unit_tests/scripts/translations/` pass locally with gettext and `SUPERSET_GETTEXT_STRICT=true`, also with Python-Unit's `-n auto --dist loadfile`. No other test in `tests/` depends on gettext. `check_translation_regression_test.py` mocks its `msgfmt` call.
The superset-pr-kit checks skipped three items: toolchain and frontend (no frontend files changed), and catalogs (the translation template did not change).
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #45245
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #45247:
URL: https://github.com/apache/superset/pull/45247#issuecomment-6150861629
## CodeAnt PR Risk: Low Risk
- The PR appears safe to merge; it installs gettext in the two CI jobs and makes missing msgcat fail the relevant tests.
- The test helpers stage the script in a temporary repository and provide Python interpreter shims, avoiding changes to real translation files.
**Assessed commit:** `256e6cb7f1b5`
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #45247:
URL: https://github.com/apache/superset/pull/45247#issuecomment-6151602858
Pretty much as you say, except for one thing: make the unit tests failing if they're ran in CI env, i.e.`os.environ.get('CI')` to be defined, and `msgcat` is not installed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "glaterza (via GitHub)" <gi...@apache.org>.
glaterza commented on PR #45247:
URL: https://github.com/apache/superset/pull/45247#issuecomment-6151886587
@hainenber Done in 482e951e05.
- `SUPERSET_GETTEXT_STRICT` is gone from the test and both workflows.
- `_require_msgcat()` fails the two msgcat tests when `CI` is set and msgcat is missing. Without `CI`, they skip as before.
- Python-Unit and Python-Nightly-Canary still install gettext, and the `python` shims stay.
Local results at 482e951e05:
```
with msgcat: 6 passed
CI=true, no msgcat: 2 failed ("CI is set, but gettext's msgcat is not on PATH: install gettext"), 4 passed
CI unset, no msgcat: 2 skipped, 4 passed
ubuntu:26.04 and debian:13: 6 passed with CI=true
```
I updated the PR description to match.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #45247:
URL: https://github.com/apache/superset/pull/45247#issuecomment-6150789117
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 256e6cb7f1b557a6ca8097c891b90b66826eb415 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6ace62eff622247ecc167446 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-45247--superset-docs-preview.netlify.app](https://deploy-preview-45247--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctBCoAgEADAv-y5JRTZoh_0DF1XEkrFDIro73mby7xQaj5KgwVgAFdt4q07xHuMak7orJMdr-JtE2xyNqxXOjEm5NiDl7LnZ_W9kGUhLSGQ1tpMwqxoMobg-wE) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-45247--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber merged PR #45247:
URL: https://github.com/apache/superset/pull/45247
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #45247:
URL: https://github.com/apache/superset/pull/45247#issuecomment-6153497085
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/45247?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 65.02%. Comparing base ([`7aadf35`](https://app.codecov.io/gh/apache/superset/commit/7aadf35e7d790ec9e79109de5686faa5b7c47527?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`482e951`](https://app.codecov.io/gh/apache/superset/commit/482e951e050b94560310de75458dac2c24d827b4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 20 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (7aadf35) and HEAD (482e951). Click for more details.
>
> <details><summary>HEAD has 4 uploads less than BASE</summary>
>
>| Flag | BASE (7aadf35) | HEAD (482e951) |
>|------|------|------|
>|python|7|4|
>|postgres|2|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #45247 +/- ##
===========================================
- Coverage 83.14% 65.02% -18.12%
===========================================
Files 3029 3037 +8
Lines 196748 197921 +1173
Branches 45967 46128 +161
===========================================
- Hits 163580 128694 -34886
- Misses 29944 66324 +36380
+ Partials 3224 2903 -321
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `35.02% <ø> (-0.02%)` | :arrow_down: |
| [mysql](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [postgres](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.09% <ø> (-0.21%)` | :arrow_down: |
| [presto](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `36.79% <ø> (-0.03%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.32% <ø> (-33.41%)` | :arrow_down: |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `52.85% <ø> (-0.20%)` | :arrow_down: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/45247/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/45247?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] test(i18n): run the babel_update.sh msgcat tests on CI [superset]
Posted by "glaterza (via GitHub)" <gi...@apache.org>.
glaterza commented on PR #45247:
URL: https://github.com/apache/superset/pull/45247#issuecomment-6151354704
@hainenber Thanks for the review. What would you prefer here?
My proposal is to remove `SUPERSET_GETTEXT_STRICT` and keep the rest:
- Python-Unit and Python-Nightly-Canary install gettext.
- The `python` and `python3` shims let the trailing-line test run where `/usr/bin` has no `python`.
- The two msgcat tests skip when msgcat is missing, as on master.
With gettext installed, both tests run and pass on CI. If a later change removes the gettext step, they skip again without a failure.
Does that work for you, or do you have another approach in mind?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org