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

[ISSUE-2113] Database Service API Integration tests #3185

Conversation

Beetelbrox
Copy link
Contributor

@Beetelbrox Beetelbrox commented Mar 6, 2022

Description

Adding the Database Service API integration tests as part of the requested improvement described on #2113. Tests for the other services will be addressed in separate PRs

Important

I had to skip a couple of test as they were not passing from what I understood was inconsistent behaviour when compared to
other entities:

  • Get reference: The implementation of get_entity_reference attempts to build the EntityReference object using the fullyQualifiedName field from the entity, but the DatabaseService entity doesn't have such field.
  • Get version: When trying to get a specific version of a database service entity the get_entity_version call fails as the DatabaseService entity object returned by the versions/<version> endpoint doesn't contain an href prop and href is a required field in the DatabaseService model.

Type of change :

  • Improvement

Checklist:

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my own.
  • I have tagged my reviewers below.
  • I have commented on my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • All new and existing tests passed.

Reviewers

@pmbrull

@pmbrull pmbrull self-requested a review March 6, 2022 10:30
@pmbrull
Copy link
Collaborator

pmbrull commented Mar 6, 2022

Hi @Beetelbrox, the errors in the CI might be related to the naming of the instances. As all tests run together, we might have some clashes if we define multiple instances with the same name. Renaming your service and db to something unique with this test might clear the issue. Thanks!

@sonarcloud
Copy link

sonarcloud bot commented Mar 6, 2022

[open-metadata-ingestion] Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Collaborator

@pmbrull pmbrull left a comment

Choose a reason for hiding this comment

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

Thanks for the work @Beetelbrox, congrats on your first PR 🚀 LGTM

@pmbrull pmbrull merged commit a6e98b3 into open-metadata:main Mar 6, 2022
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.

None yet

2 participants