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

Create DiGraph fixtured and unit test Graph Class #9871

Closed
1 task done
Tracked by #9649
QMalcolm opened this issue Apr 8, 2024 · 2 comments · Fixed by #10338
Closed
1 task done
Tracked by #9649

Create DiGraph fixtured and unit test Graph Class #9871

QMalcolm opened this issue Apr 8, 2024 · 2 comments · Fixed by #10338
Assignees

Comments

@QMalcolm
Copy link
Contributor

QMalcolm commented Apr 8, 2024

Housekeeping

  • I am a maintainer of dbt-core

Short description

As part of our Robust Testing Initiative we want to create generalized fixtures for our unit tests to make writing unit tests easier. To that end we want to add or update unit some unit tests to utilize the new fixture. Currently the Graph class is not unit tested at all. We could unit test a majority of the the class by having a DiGraph fixture. Additionally there are other parts of the code base, such as the GraphQueue which also need a DiGraph fixture. Thus creating the fixture would make it easier to create unit tests for other parts of the codebase.

Acceptance criteria

  • There is a generic DiGraph unit test fixture
  • There are unit tests for the config Graph class

Suggested Tests

The work is the tests

Impact to Other Teams

None

Will backports be required?

Narp

Context

@QMalcolm
Copy link
Contributor Author

QMalcolm commented Apr 8, 2024

I believe a graph fixture could be created by taking the manifest fixture created from a manifest fixture (perhaps the one created during #9665), and running it through a linker as is done in the compile function

@QMalcolm QMalcolm added the Refinement Maintainer input needed label Apr 8, 2024
@ChenyuLInx
Copy link
Contributor

@QMalcolm sync up with @peterallenwebb about how this ties to removing networkx(potentially).

@ChenyuLInx ChenyuLInx removed the Refinement Maintainer input needed label Apr 8, 2024
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 a pull request may close this issue.

2 participants