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

build(test): skip full python matrix for most ingest tests #1687

Merged
merged 6 commits into from
Oct 10, 2023

Conversation

ryannikolaidis
Copy link
Contributor

@ryannikolaidis ryannikolaidis commented Oct 9, 2023

We’re probably unfairly (to the test) making a large volume of new connections and requests to test services when all of our ingest tests run across the full python test matrix and when a lot of PRs a firing at once. Lets limit the full matrix run to a select few, but still have all ingest tests run on python v3.10. This is done by checking the version and skipping in ingest-test.sh.

Bonus: Bumps ingest test fixture workflow to use 3.10. This technically shouldn't make a difference, but since we're making 3.10 the default of the matrix strategy, it probably makes sense to use 3.10 for the ingest fixture generation as well for consistency.

Testing

  • example running all tests in 3.10
  • example skipping/running the expected tests in 3.8

@ryannikolaidis ryannikolaidis marked this pull request as ready for review October 9, 2023 20:23
echo "--------- RUNNING SCRIPT $script --- IGNORING FAILURES"
python_version=$(python --version 2>&1)

for test in "${all_tests[@]}"; do
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add some documentation here on what this logic is meant to be handling? It's getting pretty complicated and reading bash logic comparisons are not the easiest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, totally fair. assuming this is about the conditional logic a few lines down, right? just dropped a comment on it, let me know if that covers it!

@rbiseck3
Copy link
Contributor

rbiseck3 commented Oct 9, 2023

Small ask for some additional documentation, but otherwise looks good. Always a fan of anything that helps expedite the ingest tests.

@ryannikolaidis ryannikolaidis added this pull request to the merge queue Oct 10, 2023
Merged via the queue into main with commit 3e101d3 Oct 10, 2023
@ryannikolaidis ryannikolaidis deleted the ryan/full-test-connectors branch October 10, 2023 17:08
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.

2 participants