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.

Sign up for GitHub

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

Jump to bottom

ci improvements #1159

Merged
merged 26 commits into from
Apr 7, 2024
Merged

ci improvements #1159

merged 26 commits into from
Apr 7, 2024

Conversation

sh-rp
Copy link
Collaborator

@sh-rp sh-rp commented Mar 28, 2024 •
edited
Loading

Description

This PR will include improvements to our ci.

NOTE: I can't see the athena iceberg tests being executed, I don't get what is going on there..

Details

  • Removes unneeded matrix job collectors for destination tests
  • Cleans up naming of workflows and job to produce more legible output
  • Only runs tests marked as "essential" for most destination on PRs
  • Add the possibility to run all tests with a "ci-full" tag on the PR
  • Add the possibility to run tests on a fork if tag "ci from fork" is defined
  • there is a nightly schedule at 2am where the full test suite is run

To Do

  • Update the required tests on the repo to not point to the matrix collection tests

Other fixes

  • Snippets tests also need secrets, so they are now also prevented to run on forks by default

To discuss

  • Local tests require the secrets, although they shouldn't really. there are some filesystem tests that use gdrive. For now they are also not disabled on forks. we should talk about it.

Ideas

  • Run a simple load for all destinations on all OSes to see that the drivers work everywhere?

@netlify Netlify
Copy link

netlify bot commented Mar 28, 2024 •
edited
Loading

✅ Deploy Preview for dlt-hub-docs ready!

Name Link
🔨 Latest commit 02aea29
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6612428dcd52e300085a4169
😎 Deploy Preview https://deploy-preview-1159--dlt-hub-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sh-rp sh-rp force-pushed the d#/ci_improvements branch from e833e7b to 053ea3a Compare March 28, 2024 15:55
@sh-rp sh-rp marked this pull request as ready for review April 2, 2024 11:47
@sh-rp sh-rp force-pushed the d#/ci_improvements branch from f99ce40 to bd141d8 Compare April 2, 2024 17:40
@sh-rp sh-rp added the ci full run the full load tests on pr label Apr 2, 2024
@sh-rp sh-rp removed the ci full run the full load tests on pr label Apr 2, 2024
rudolfix
rudolfix requested changes Apr 3, 2024
View reviewed changes
Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

  1. you can mark whole modules: https://docs.pytest.org/en/7.1.x/example/markers.html#marking-whole-classes-or-modules
  2. I'd also test
  • test_job_client
  • test_insert_job_client
  1. load/pipelines/test_pipeline.py
  2. load/pipelines/test_arrow_loading

up to you but I would mark all destination specific tests as essential

  • all folders in tests/load/... testing specific destinations
  • same for tests/load/pipelines

.github/workflows/test_destination_athena_iceberg.yml Outdated
echo "One or more matrix job tests failed or were cancelled. You may need to re-run them." && exit 1
name: Run all tests Linux
if: ${{ contains(github.event.pull_request.labels.*.name, 'ci full') || github.event_name == 'schedule'}}

Copy link
Collaborator

Choose a reason for hiding this comment

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

delete lines

.github/workflows/test_destination_snowflake.yml Show resolved Hide resolved
@sh-rp
Copy link
Collaborator Author

sh-rp commented Apr 3, 2024

note: make mssql run fully each time

@sh-rp sh-rp force-pushed the d#/ci_improvements branch from 0ca53f8 to 9b08e8c Compare April 3, 2024 10:01
@sh-rp sh-rp changed the title [do not merge] testing ci improvements testing ci improvements Apr 3, 2024
@sh-rp sh-rp changed the title testing ci improvements ci improvements Apr 3, 2024
@sh-rp
Merge branch 'devel' into d#/ci_improvements
c19ca85 
# Conflicts:
#	.github/workflows/test_doc_snippets.yml
@sh-rp sh-rp linked an issue Apr 3, 2024 that may be closed by this pull request
[chores] split long running destination tests for nightly builds #997
Closed
rudolfix
rudolfix previously approved these changes Apr 7, 2024
View reviewed changes
Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

this is so good! @sh-rp problems with athena and synapse were found and fixed in the other branches

@rudolfix rudolfix merged commit e0b4731 into devel Apr 7, 2024
15 checks passed
@rudolfix rudolfix deleted the d#/ci_improvements branch April 7, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sultaniman sultaniman sultaniman left review comments

@rudolfix rudolfix rudolfix left review comments

Assignees

@sh-rp sh-rp

Labels
None yet
Projects
dlt core library
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[chores] split long running destination tests for nightly builds
3 participants
@sh-rp @sultaniman @rudolfix