Skip to content

Conversation

pnilan
Copy link
Contributor

@pnilan pnilan commented May 22, 2025

What

  • Fixes pytest-fast poe command by specifying unit_tests/ directory in command

Summary by CodeRabbit

  • Chores
    • Updated test configuration to run only unit tests when using the fast test command.

@github-actions github-actions bot added bug Something isn't working security labels May 22, 2025
@pnilan pnilan marked this pull request as ready for review May 22, 2025 20:48
@Copilot Copilot AI review requested due to automatic review settings May 22, 2025 20:48
@pnilan pnilan force-pushed the pnilan/fix/poe-pytest-fast branch from b8bb7d9 to 69b50db Compare May 22, 2025 20:49

This comment was marked as outdated.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b73b46 and 69b50db.

📒 Files selected for processing (1)
  • pyproject.toml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
  • GitHub Check: Check: 'source-amplitude' (skip=false)
  • GitHub Check: Check: 'source-hardcoded-records' (skip=false)
  • GitHub Check: Check: 'source-pokeapi' (skip=false)
  • GitHub Check: Check: 'source-shopify' (skip=false)
  • GitHub Check: Pytest (All, Python 3.10, Ubuntu)
  • GitHub Check: Pytest (All, Python 3.11, Ubuntu)
  • GitHub Check: SDM Docker Image Build
  • GitHub Check: Pytest (Fast)
  • GitHub Check: Analyze (python)

Copy link
Contributor

@Copilot Copilot AI left a comment

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 refines the test tooling and enhances config migration by propagating a config_path through declarative source constructors.

  • Updates the pytest-fast task to target the unit_tests/ folder.
  • Introduces and wires a config_path parameter into all declarative source and CLI entrypoints.
  • Adjusts the Spec and ManifestDeclarativeSource classes to migrate & transform configs based on config_path.

Reviewed Changes

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

Show a summary per file
File Description
pyproject.toml Change pytest-fast command to specify unit_tests directory.
unit_tests/sources/declarative/test_manifest_declarative_source.py Add fixtures and tests for config migration via config_path.
unit_tests/sources/declarative/spec/test_spec.py Remove now-duplicated migration tests in spec tests.
airbyte_cdk/sources/declarative/yaml_declarative_source.py Add config_path parameter to __init__.
airbyte_cdk/sources/declarative/spec/spec.py Simplify migrate_config signature; remove args/source.
airbyte_cdk/sources/declarative/manifest_declarative_source.py Integrate _migrate_and_transform_config using config_path.
airbyte_cdk/sources/declarative/concurrent_declarative_source.py Propagate config_path to superclass.
airbyte_cdk/cli/source_declarative_manifest/_run.py Add config_path to CLI run entrypoint.
Comments suppressed due to low confidence (1)

pyproject.toml:173

  • The updated pytest-fast command removes the --skipslow flag, which the PR description indicates should be preserved. Consider adding --skipslow before the marker expression to skip slow tests as intended.
pytest-fast = {cmd = "poetry run coverage run -m pytest unit_tests --durations=5 --exitfirst -m 'not flaky and not slow and not requires_creds'", help = "Run pytest tests, failing fast and excluding slow tests."}

@pnilan pnilan merged commit 155cdc8 into main May 22, 2025
26 of 29 checks passed
@pnilan pnilan deleted the pnilan/fix/poe-pytest-fast branch May 22, 2025 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant