apache / apache/airflow

Add missing tests for `sftp` exceptions

Open
#72,268 1 comment 0 reactions 0 assignees View on GitHub
area:providers good first issue provider:sftp
Dominant language
Python
Stars
46.9k
Forks
17.8k
Avg merge
2d 9h
Merged PRs (30d)
472

Description

## Description

One module in the `sftp` provider has no dedicated test module, and (unlike most entries on the `OVERLOOKED_TESTS` list) it is not covered indirectly. Nothing under any `providers/*/tests/` directory imports it at all:

| Module | Expected test file |
| --- | --- |
| `providers/sftp/src/airflow/providers/sftp/exceptions.py` | `providers/sftp/tests/unit/sftp/test_exceptions.py` |

It is currently suppressed in the `OVERLOOKED_TESTS` allowlist in [`airflow-core/tests/unit/always/test_project_structure.py`](https://github.com/apache/airflow/blob/main/airflow-core/tests/unit/always/test_project_structure.py).

This is a scoped subset of the meta issue #35442, limited to the `sftp` provider.

### What should these tests cover?

The module defines a single exception, `ConnectionNotOpenedException`. Testing the class declaration on its own would be close to worthless, so the useful scope here is the behaviour that raises it, which turns out to be uncovered as well. The only raise site is the `handle_connection_management` decorator in `providers/sftp/src/airflow/providers/sftp/hooks/sftp.py`, and no test in `providers/sftp/tests/` currently exercises that branch.

Worth covering:

- a `handle_connection_management` wrapped hook method called with `use_managed_conn` false and no open connection raises `ConnectionNotOpenedException`, and the message points the caller at `hook.get_managed_conn()`
- the same call with a connection already open delegates through to the wrapped function and returns its result
- with `use_managed_conn` true the decorator opens a managed connection rather than raising, and the connection is set on the hook for the duration of the call
- `ConnectionNotOpenedException` derives from `AirflowException`, which matters because callers catching the base class rely on that and narrowing it later would be a breaking change for them

The decorator can be driven against a small stub hook with a mocked connection, so no SFTP server is needed.

### Definition of done

1. Add the test module at the path in the table above.
2. Remove the corresponding entry from `OVERLOOKED_TESTS` in `airflow-core/tests/unit/always/test_project_structure.py`
3. This test should pass:

```bash
breeze testing providers-tests providers/sftp/tests/unit/sftp/test_exceptions.py
```

---
Drafted-by: Claude Code (Opus 5); reviewed and edited by @jroachgolf84 before posting.

Contributor guide

Open the contributing guide

Research direction

Start with providers/sftp/src/airflow/providers/sftp/exceptions.py and the handle_connection_management decorator in providers/sftp/src/airflow/providers/sftp/hooks/sftp.py. Add providers/sftp/tests/unit/sftp/test_exceptions.py using a stub hook and mocked connection to cover the listed branches and exception inheritance. Remove the entry from airflow-core/tests/unit/always/test_project_structure.py, then run breeze testing providers-tests providers/sftp/tests/unit/sftp/test_exceptions.py.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.