apify / crawlee-python

Crawlee—A web scraping and browser automation library for Python to build reliable crawlers. Extract data for AI, LLMs, RAG, or GPTs. Download HTML, PDF, JPG, PNG, and other files from websites. Works with BeautifulSoup, Playwright, and raw HTTP. Both headful and headless mode. With proxy rotation.
https://crawlee.dev/python/
Apache License 2.0
4.65k stars 319 forks source link

chore: add tests for processing request payload #688

Closed Mantisus closed 1 week ago

Mantisus commented 1 week ago

Description

Correction of comments related to PR #683 Add new tests for http_crawler different forms of payload

Testing

The test checks that the payload on the client side matches the data received by the server. Also added verification that different types of payload are correctly handled on the client side and correctly recognized by the server with appropriate headers

Checklist

vdusek commented 1 week ago

Nice! One suggestion: the test_sending_payload, test_sending_form_payload, and test_sending_json_payload functions share quite a lot of the same code. We could at least use the payload as a fixture.

@pytest.fixture
def payload() -> dict[str, str]:
    return {
        'custname': 'John Doe',
        'custtel': '1234567890',
        'custemail': 'johndoe@example.com',
        'size': 'large',
        'topping': '["bacon", "cheese", "mushroom"]',
        'delivery': '13:00',
        'comments': 'Please ring the doorbell upon arrival.',
    }
B4nan commented 1 week ago

fix: adding tests for testing payload

Addings tests is not a fix. Looking at your code, this PR is not just adding tests. Pay more attention to the PR titles, they will end up in the changelog, so they need to make sense to our users. If its not something to include in changelog, don't use fix commit type.

janbuchar commented 1 week ago

Nice! One suggestion: the test_sending_payload, test_sending_form_payload, and test_sending_json_payload functions share quite a lot of the same code. We could at least use the payload as a fixture.

@pytest.fixture
def payload() -> dict[str, str]:
    return {
        'custname': 'John Doe',
        'custtel': '1234567890',
        'custemail': 'johndoe@example.com',
        'size': 'large',
        'topping': '["bacon", "cheese", "mushroom"]',
        'delivery': '13:00',
        'comments': 'Please ring the doorbell upon arrival.',
    }

I'd say a fixture is overkill in this case, a top-level constant (PAYLOAD = {...}) would work just as fine