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

Add DatasetDao tests #314

Closed
wslulciuc opened this issue Feb 13, 2019 · 6 comments
Closed

Add DatasetDao tests #314

wslulciuc opened this issue Feb 13, 2019 · 6 comments
Assignees
Milestone

Comments

@wslulciuc
Copy link
Member

wslulciuc commented Feb 13, 2019

Our test coverage for DatasetDao is currently < 25%, let's add some tests to get it closer to 75%

codecov link: https://codecov.io/gh/MarquezProject/marquez/src/master/src/main/java/marquez/db/DatasetDao.java

@wslulciuc wslulciuc added this to the 0.2.0 milestone Feb 13, 2019
@wslulciuc wslulciuc added the db label Feb 18, 2019
@hjpatel16
Copy link
Contributor

Hey, I would like to work on it

@wslulciuc
Copy link
Member Author

Hey @hjpatel16. Great to hear you'd like to help out! I'm actually fixing this issue in PR #395 (sorry, should've assigned this to myself sooner). Once merged into master, it will serve as an example on how to write tests for our DAO classes.

Is there an issue tagged with "good first issue" you'd like to pick up?

@wslulciuc wslulciuc self-assigned this Mar 18, 2019
@hjpatel16
Copy link
Contributor

Actually, I was looking more than "good first issue" where I would be able to code more and contribute more. Let me know if you know any specific issue which will be perfect for me

@wslulciuc
Copy link
Member Author

@hjpatel16, sure! How does adding a factory method to our response models sound? Full details can be found here #309. I know the issues related to the epic are labeled as "good first issue", but these changes are non-trivial and would require writing tests, with new methods used right away in our resource classes.

Let me know if that's something you're interested in.

NOTE: You would need a separate PR per issue in the epic

@hjpatel16
Copy link
Contributor

Yes, I would like to work on that epic. Thanks @wslulciuc, much appreciated

@wslulciuc
Copy link
Member Author

Fixed #395

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

No branches or pull requests

2 participants