galaxyproject / galaxyproject/pulsar

9 tests in 3 files have never run under pytest — filenames don't match default python_files

Open
#484 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
46
Forks
62
Avg merge
3d 15h
Merged PRs (30d)
14

Description

> **Posted by Claude (AI assistant) on behalf of jmchilton** — investigated and written by Claude, not authored by jmchilton personally.

Three test files in `test/` are never collected by pytest, so **9 tests have not run in CI since the nose→pytest migration in February 2021**. They are not skipped and not deselected — pytest never looks at them, so nothing in any report indicates they exist.

## The files

| File | Tests | Added |
|---|---|---|
| `test/integration_test_state.py` | 6 | 2018-02-19 |
| `test/integration_test_cli_submit.py` | 2 | 2015-04-07 |
| `test/cli_help_tests.py` | 1 | 2015-04-17 |

`test/integration_test_state.py` is the significant one. It covers restart/recovery and terminal-status notification:

- `test_restart_finishes_job`
- `test_recovery_failure_fires_lost_status`
- `test_staging_failure_fires_failed_status`
- `test_async_request_of_mq_status`
- `test_async_request_of_mq_status_lost`
- `test_setup_failure_fires_failed_status`

## Why they aren't collected

`pytest.ini` sets only `log_level`:

```ini
[pytest]
log_level = DEBUG
```

With no `python_files`, pytest's defaults apply: `test_*.py` and `*_test.py`. None of the three filenames match either pattern — `integration_test_state.py` ends in `_state.py`, `integration_test_cli_submit.py` in `_submit.py`, and `cli_help_tests.py` in `_tests.py` (plural).

`test/integration_test.py` *does* match `*_test.py`, which is why DRMAA integration tests do appear in CI logs and give the impression the directory is covered.

## Root cause

Before commit `1da5d6d` ("Catch drmaa import RuntimeErrors, swap nose for pytest", 2021-02-02), `tox.ini` ran:

```ini
commands = nosetests [] # full env
commands = nosetests --verbose --exclude '.*integration.*' [] # unit envs
```

nose's default `testMatch` is `(?:^|[\b_\.-])[Tt]est`, which matches a filename containing `_test` or `_tests` anywhere — so all three files *were* collected under nose. The explicit `--exclude '.*integration.*'` on the unit envs is itself evidence the full env was picking up the integration files, otherwise the exclusion would have been unnecessary.

That commit replaced the above with:

```ini
commands = pytest -v --log-level=debug
commands = pytest -v --log-level=debug --ignore-glob='*integration*.py'
```

The `--ignore-glob` faithfully preserved the intent of the nose `--exclude` for the unit envs, which is likely why the change looked complete. But pytest's narrower default `python_files` meant the full env silently stopped collecting the three files, and nothing failed to signal it.

## Evidence

From a current `Run Tests (test-ci, 3.11)` job log:

```
collecting ... collected 342 items / 8 deselected / 334 selected
...
==== 270 passed, 64 skipped, 8 deselected, 54 warnings in 124.08s ====
```

`270 + 64 = 334`, so every selected test ran, and the log contains **zero** occurrences of the string `integration_test_state`. The file contributes to none of the collected, skipped, or deselected counts.

## Why it surfaced now

This came up while reviewing #482, which changes the DRMAA `JobState.FAILED` → Pulsar status mapping. `test_restart_finishes_job` asserts a status of `complete` for a job killed directly through the DRMAA session, and `test_staging_failure_fires_failed_status` / `test_setup_failure_fires_failed_status` cover exactly the terminal-failure notification path that change affects. Those tests would have been informative about the change; because they never run, CI on that PR is green and carries no signal either way.

## Suggested fix

Extend the collection patterns in `pytest.ini`:

```ini
[pytest]
log_level = DEBUG
python_files = test_*.py *_test.py integration_test_*.py cli_help_tests.py
```

Renaming the files to `*_test.py` would also work and is arguably tidier, but it breaks any external references and loses the `--ignore-glob='*integration*.py'` filtering that the unit envs still rely on — `integration_test_state.py` must keep `integration` in its name for `test-unit` to continue excluding it correctly.

**Expect newly-collected tests to need work before they go green.** They have not executed in over five years and reference behaviour that may have changed; `test_restart_finishes_job` in particular asserts an expectation that looks incorrect independent of this issue. Turning them on will likely be its own PR rather than a one-line config change, and a first step might be to run them locally and triage:

```sh
pytest -v test/integration_test_state.py test/integration_test_cli_submit.py test/cli_help_tests.py
```

(These paths collect fine when named explicitly — it is only directory-scan collection that skips them.)

## Possible follow-up

A CI guard that fails when a file under `test/` defines tests but is not collected would prevent a silent recurrence. A cheap version compares `pytest --collect-only -q` output against a grep for `def test` across `test/`.

Contributor guide

Open the contributing guide

Research direction

Start with pytest.ini and the three named files: test/integration_test_state.py, test/integration_test_cli_submit.py, and test/cli_help_tests.py. Run pytest -v on those paths, then inspect directory collection and the existing CI commands. Done means the files are collected by the intended test environment and any newly exposed failures are identified rather than silently omitted.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
ci-cd, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.