Stale Access application path-param tests expect valid account_id to raise

Open Beginner friendly
#2,747 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
88/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
api, testing

Research direction

Start with the skipped path-param tests in tests/api_resources/zero_trust/access/test_applications.py, especially test_path_params_create_overload_1 and the sync and async test_path_params_list cases. Compare their expectations with src/cloudflare/resources/zero_trust/access/applications/applications.py, then run the focused application tests. Done means a non-empty account_id is accepted and only empty or missing account and zone identifiers raise errors.

Written by the indexing model from the issue text.

Description

Confirm this is a Python library issue and not an underlying Cloudflare API issue.
  • This is an issue with the Python library
Describe the bug

Some skipped path-param tests for zero_trust.access.applications appear to be stale after the account/zone path selection logic was fixed.

For example, these tests still expect ValueError("You must provide either account_id or zone_id") when account_id="account_id" is provided:

  • test_path_params_create_overload_1 in tests/api_resources/zero_trust/access/test_applications.py
  • test_path_params_list in both sync and async sections of the same file

That expectation now contradicts the current resource implementation, which correctly treats account_id as sufficient and builds an /accounts/{account_id}/... path. This looks related to the behavior discussed in #2705, where passing account_id should not require zone_id.

I noticed this while checking #2705. The runtime behavior on current main looks correct, but the skipped generated tests still encode the old/broken expectation.

To Reproduce
  1. Inspect the current main version of tests/api_resources/zero_trust/access/test_applications.py.
  2. See the valid-account_id branches in these tests:
  3. Compare with the current implementation in src/cloudflare/resources/zero_trust/access/applications/applications.py, where account_id selects the accounts path and does not need zone_id.
  4. A minimal local smoke check confirms the current behavior builds GET /accounts/acct_123/access/apps for client.zero_trust.access.applications.list(account_id="acct_123").

Expected: the path-param tests should only expect an error for empty/missing account and zone identifiers, not for a non-empty account_id.

Code snippets
client.zero_trust.access.applications.list(account_id="acct_123")
# current main builds: GET /accounts/acct_123/access/apps
OS

all

Python version

3.11.1

Library version

main / 5.7.0

Dominant language
Python
Stars
509
Forks
150
Avg merge
3h 15m
Merged PRs (30d)
1

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from cloudflare/cloudflare-python

All issues in cloudflare/cloudflare-python

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.