Skip to content

fix(image_search): validate engine parameter and raise ValueError for unsupported values - #97

Merged
vedaant00 merged 2 commits into
mldsveda:mainfrom
HeaTTap:fix/validate-image-search-engine
Aug 5, 2026
Merged

fix(image_search): validate engine parameter and raise ValueError for unsupported values#97
vedaant00 merged 2 commits into
mldsveda:mainfrom
HeaTTap:fix/validate-image-search-engine

Conversation

@HeaTTap

@HeaTTap HeaTTap commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #94

Changes

  • Updated ImageSearchScraper.scrape and ImageSearchScraper.scrape_async to validate that engine is either "bing" or "google".
  • Raises a descriptive ValueError if an unsupported engine string (e.g., "googel") is provided instead of silently defaulting to Bing.
  • Added unit tests for both sync and async paths in tests/test_scrapers/test_other.py to verify ValueError is raised as expected.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes issue #94 by making ImageSearchScraper reject unsupported engine values instead of silently falling back to Bing, and adds regression tests to lock in the behavior.

Changes:

  • Validate engine in ImageSearchScraper.scrape and scrape_async and raise ValueError for unsupported values.
  • Add sync + async unit tests asserting ValueError is raised for an invalid engine string.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/pyscrappy/scrapers/image_search.py Adds explicit engine validation and errors for unsupported engines in both sync and async paths.
tests/test_scrapers/test_other.py Adds sync/async tests verifying unsupported engines raise ValueError.
Suppressed comments (2)

src/pyscrappy/scrapers/image_search.py:82

  • ValueError messages in other scrapers start with a capital letter (e.g., src/pyscrappy/scrapers/news.py:78, src/pyscrappy/scrapers/spotify.py:66). For consistency, consider capitalizing this message too.
        if engine not in ("bing", "google"):
            raise ValueError(f"unsupported engine {engine!r}; use 'bing' or 'google'")

tests/test_scrapers/test_other.py:232

  • If the ValueError message is capitalized for consistency, this test's regex should be updated to match the new casing.
        with pytest.raises(ValueError, match="unsupported engine 'googel'"):
            await scraper.scrape_async(query="test", engine="googel")

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/pyscrappy/scrapers/image_search.py Outdated
Comment thread tests/test_scrapers/test_other.py Outdated
Match the sibling scrapers' ValueError casing (news/imdb/youtube start with
a capital), update the test regexes to match, and remove the trailing blank
line ruff format flagged.
@vedaant00
vedaant00 merged commit 8ab0c42 into mldsveda:main Aug 5, 2026
6 checks passed
@vedaant00

Copy link
Copy Markdown
Collaborator

Correct fix and good sync+async coverage, thanks @HeaTTap. Pushed a tiny follow-up: capitalized the ValueError to match the other scrapers (news/imdb/youtube) and dropped a trailing blank line for the format check. Green now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate the image-search engine (unknown engine silently runs Bing)

3 participants