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

feat: dbt test by subprocess #108

Closed
wants to merge 29 commits into from
Closed

Conversation

virvirlopez
Copy link
Contributor

@virvirlopez virvirlopez commented Feb 27, 2021

Description

Adding the logic of running the tests from DBT. We are using subprocess to run directly the test from DBT and then parse the response.

How was the change tested

Ran it in local.

Ticket

Closes #65

Checklist

(Ideally, all boxes are checked by the time we merged the PR, if you don't know how to do any of these don't hesitate to say so in the PR and we'll help you out.)

  • I formatted my PR name according to CONTRIBUTING.md
  • I added a news fragment to help populating the changelog as encouraged in CONTRIBUTING.md
  • I added "Closes #<issue_number>" in the "Issue Information" section (if no issue, feel free to tick thick the box anyway).

@codecov
Copy link

codecov bot commented Feb 27, 2021

Codecov Report

Merging #108 (37a5578) into main (acf19ee) will decrease coverage by 0.43%.
The diff coverage is 59.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
- Coverage   84.71%   84.27%   -0.44%     
==========================================
  Files          17       17              
  Lines         811      833      +22     
==========================================
+ Hits          687      702      +15     
- Misses        124      131       +7     
Flag Coverage Δ
unittests 84.27% <59.25%> (-0.44%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dbt_sugar/core/task/doc.py 59.15% <59.25%> (+1.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acf19ee...37a5578. Read the comment docs.

@virvirlopez virvirlopez marked this pull request as ready for review February 27, 2021 16:54
Copy link
Member

@bastienboutonnet bastienboutonnet left a comment

Choose a reason for hiding this comment

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

Really awesome that we can do this! I have a few very small suggestions and then we should be able to merge it.

I'll be nice to test it with some non-built in tests when we merged (we might have to add some packages in the toy dbt project.

Also, we're going to have to require dbt to be in the package's main requirements.

You can add it to the requirements by doing the following:
poetry add dbt and it will add the entry in the pyproject.toml if you're not sure let me know we can look together.

Finally if you can reference the issue it closes and write a news fragment for the changeling that would be super awesome.

dbt_sugar/core/task/doc.py Outdated Show resolved Hide resolved
dbt_sugar/core/task/doc.py Outdated Show resolved Hide resolved
dbt_sugar/core/task/doc.py Outdated Show resolved Hide resolved
dbt_sugar/core/task/doc.py Outdated Show resolved Hide resolved
dbt_sugar/core/task/doc.py Outdated Show resolved Hide resolved
@virvirlopez virvirlopez force-pushed the feat/dbt_test_by_subprocess branch from 168cd5b to 76800da Compare March 6, 2021 14:30
@virvirlopez virvirlopez force-pushed the feat/dbt_test_by_subprocess branch from 6798e97 to 4fe0737 Compare March 6, 2021 14:39
Virginia Lopez-Gil and others added 12 commits March 6, 2021 15:46
* feat: implement test and tags deactivation

* Add test and tag skipping to CLI + update tests

* Fix: test with argparse (#102)

* implement config overriding from cli args for tests and tags

* keep schema.yml undocumented

* remove extra description entry commited by mistake

* add news fragment

Co-authored-by: Virginia Lopez-Gil <vlopezgil@tripactions.com>
Co-authored-by: virvirlopez <virbyte@gmail.com>
Bumps [pydantic](https://github.com/samuelcolvin/pydantic) from 1.7.3 to 1.8.
- [Release notes](https://github.com/samuelcolvin/pydantic/releases)
- [Changelog](https://github.com/samuelcolvin/pydantic/blob/master/HISTORY.md)
- [Commits](pydantic/pydantic@v1.7.3...v1.8)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [rich](https://github.com/willmcgugan/rich) from 9.10.0 to 9.12.3.
- [Release notes](https://github.com/willmcgugan/rich/releases)
- [Changelog](https://github.com/willmcgugan/rich/blob/master/CHANGELOG.md)
- [Commits](Textualize/rich@v9.10.0...v9.12.3)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [pydantic](https://github.com/samuelcolvin/pydantic) from 1.8 to 1.8.1.
- [Release notes](https://github.com/samuelcolvin/pydantic/releases)
- [Changelog](https://github.com/samuelcolvin/pydantic/blob/master/HISTORY.md)
- [Commits](pydantic/pydantic@v1.8...v1.8.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
* Fix: test with argparse (#102)

* implement config overriding from cli args for tests and tags

* keep schema.yml undocumented

* give an empty default value to schema in flags

* refactor calls to DbtProfile to take flags

* add FIXME/question on doc for dbt credentials reading

* rename flag_dict to flag_override_dict to be more explicit

* update tests

* remove extra description field in schema.yml

Co-authored-by: virvirlopez <virbyte@gmail.com>
Virginia Lopez-Gil added 2 commits March 13, 2021 14:10
@virvirlopez virvirlopez closed this May 3, 2021
@virvirlopez virvirlopez deleted the feat/dbt_test_by_subprocess branch May 3, 2021 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants