Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Rework the hook system #1673

Closed
Stranger6667 opened this issue Jan 30, 2023 · 0 comments
Closed

[FEATURE] Rework the hook system #1673

Stranger6667 opened this issue Jan 30, 2023 · 0 comments
Assignees
Labels
Component: Hooks Extensibility and customization Difficulty: Hard Complex, needs deep understanding Priority: Medium Planned for regular releases Type: Feature New functionalities or enhancements UX: Usability Enhances user experience
Milestone

Comments

@Stranger6667
Copy link
Member

Stranger6667 commented Jan 30, 2023

Problem

Applying hooks for different operations is clunky:

@schemathesis.hook
def before_generate_case(context, strategy):
    op = context.operation

    def tune_case(case):
        if op.method == "PATCH" and op.path == "/users/{user_id}/":
            case.path_parameters["user_id"] = case.body["data"]["id"]
        if op.method == "GET" and op.path == "/users/{user_id}/":
            case.path_parameters["user_id"] = 42
        return case

    return strategy.map(tune_case)

All the dispatching is in the same place and all the logic for different endpoints is also there.

Solution

Filters

Extend the hook system to allow users to specify the scope of hook functions through a decorator instead of implementing conditions inside the hook function body. This would simplify the filtering process and make it easier to use.

Naming & semantic change

The hook names are currently confusing and too verbose, making them difficult to use comfortably. The suggestion is to simplify the hook names to make them clearer and easier to use. This could include options such as {map,filter,flatmap}_{query,path_parameters,cookie,header,body,case} to match the Hypothesis' semantics:

# Filter via custom function
def select_operations(context):
    # Or any complex logic here
    return context.operation.method == "PATCH" and context.operation.path == "/users/{user_id}/"
    

# Apply hook only if `select_operations` returned `True`
@schemathesis.hook.apply_to(select_operations)
def filter_query(context, query):
    return query["key"] != "42"


# Apply hook to everything except `GET /users/{user_id}/`
@schemathesis.hook.skip_for(method="GET", path="/users/{user_id}/")
def map_path_parameters(context, path_parameters):
    path_parameters["user_id"] = 42
    return path_parameters


# Simple filter hook for all operations
@schemathesis.hook
def filter_query(context, query):
    return query["key"] != "42"

Filtration options are apply_to / skip_for

However, we need to be clear about the order in which these hooks are applied to avoid confusion.

The current hooks that accept strategies could be renamed to generate_{query,path_parameters,cookie,header,body,case} because the before prefix is a bit misleading.

Other considerations

Testing

It might be not really convenient to test hooks now, but we can expose some helpers:

schema = ...

@schemathesis.hook
def filter_query(context, query):
    # Some complex logic here
    return query["key"].count("0") < 5


@schemathesis.hook
def map_header(context, headers):
    if "X-Condition" in headers:
        headers["X-Value"] = "45"
    return headers


@schema.hooks.given(filter_query)
def test_filter_hook(query):
    # `query` is a randomly generated value passing through the hook (with filters applied)
    assert query["key"].count("0") < 5


@schema.hooks.given(map_header)
def test_map_hook(headers):
    assert "X-Value" in headers
    assert headers["X-Value"] == 42

Schemathesis basically does something similar to test built-in hooks. Exposing a cleaned-up version would benefit other people who develop hooks.

Validation

Validate hooks input and let the user know about some configuration issues immediately:

@schemathesis.hook.skip_for()
def filter_query(context, query):
    ...

InvalidConfiguration: The "filter_query" hook defined at hooks.py:42 does not exclude anything and therefore has no effect

If a hook was not applied at all (some wrong filters), then we can report it as a warning at the end of the test run:

WARNING: The `filter_query` hook did not match any API operation. Re-check your filters

Debugging

It would be handy to see the filter result:

@schemathesis.hook
def filter_query(context, query):
    ...


operations = schema.filter_operations(filter_query.filters)
# `APIOperation` instances
# GET /users/
# POST /users/
# ...

Such a feature would actually be really helpful for other filter cases (like the ones planned in #1256) as sometimes users report that filters they input do not match what they wanted and having a way to debug it would be really helpful.

Additionally, we may have some debug flag that enables extra output from a hook:

@schemathesis.hook(debug=True)
def filter_query(context, query):
    ...

# Outputs something like
# `format_query`: PASS   {"key": 42}
# `format_query`: MISS   {"key": 45}
# PASS 30 (30%)
# MISS 70 (70%)

# And for `map_query` it could be
# `map_query`: {"key": 42} -> {"key": 0}

But I am not sure where to output this info.

We might want to customize texts for exceptions that happen inside hooks to provide more concise info to the user:

@schemathesis.hook
def filter_query(context, query):
    1 / 0

Exception happened in the "filter_query" hook defined in hooks.py:42 + show only relevant stack frames to avoid visual clutter.

Documentation

This change surely needs a full rewrite with way more examples + a migration guide from old hooks.

More thoughts

  • Execution order. Now the execution order is the reverse order of declaration. Maybe we can try something like pluggy does and apply some extra arguments (like first / last) to control it. Now sure whether it will simplify the API though.
  • Complex conditions. Joining filter conditions via or / and - it might help for some cases, but probably will complicate the API.
  • Python compatibility. Would such decorators work on Python < 3.9?
  • Backwards compatibility. Any issues with how the current hooks might be affected?
  • Performance. Are any performance pitfalls with the new approach?
@Stranger6667 Stranger6667 added Status: Needs Triage Requires initial assessment to categorize and prioritize Type: Feature New functionalities or enhancements labels Jan 30, 2023
@Stranger6667 Stranger6667 self-assigned this Jan 30, 2023
@Stranger6667 Stranger6667 changed the title [FEATURE] Filter for hooks [FEATURE] Rework the hook system Feb 7, 2023
@Stranger6667 Stranger6667 added Type: Enhancement and removed Status: Needs Triage Requires initial assessment to categorize and prioritize Type: Feature New functionalities or enhancements labels Feb 7, 2023
@Stranger6667 Stranger6667 added this to the Schemathesis 4.0 milestone Feb 12, 2023
Stranger6667 pushed a commit that referenced this issue Oct 3, 2023
Stranger6667 pushed a commit that referenced this issue Oct 4, 2023
Stranger6667 pushed a commit that referenced this issue Oct 4, 2023
@Stranger6667 Stranger6667 added Status: Needs Triage Requires initial assessment to categorize and prioritize and removed Type: Enhancement labels Oct 11, 2023
Stranger6667 pushed a commit that referenced this issue Oct 11, 2023
@Stranger6667 Stranger6667 added Priority: Medium Planned for regular releases Type: Feature New functionalities or enhancements Difficulty: Hard Complex, needs deep understanding UX: Usability Enhances user experience Component: Hooks Extensibility and customization and removed Status: Needs Triage Requires initial assessment to categorize and prioritize labels Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Hooks Extensibility and customization Difficulty: Hard Complex, needs deep understanding Priority: Medium Planned for regular releases Type: Feature New functionalities or enhancements UX: Usability Enhances user experience
Projects
None yet
Development

No branches or pull requests

1 participant