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

[FDS-2294] Run schematic_api tests #1476

Merged
merged 8 commits into from
Aug 28, 2024
Merged

Conversation

BryanFauble
Copy link
Collaborator

@BryanFauble BryanFauble commented Aug 27, 2024

Problem:

  1. schematic_api tests were being ignored.
  2. One of the integration tests was out of date for the new logic.
  3. There are issues with the tangled_tree logic that was causing test failures. It was tracked down, context was added to the comment: [FDS-2294] Run schematic_api tests #1476 (comment)

Solution:

  1. unignore schematic_api tests
  2. Ignore the benchmark tests which are long-running tests that should not really be included with the normal integration test runs
  3. Update the logic around tangled_tree.

Testing:

  1. I verified the logic with the return of the API data as well as manually via breakpoints and inspecting the data.
  2. Will wait for a fully passing integration test suite.

image

Related to this ticket: FDS-2294

Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

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

🔥LGTM - the code looks good, but ill leave it to FAIR team for the logic

@BryanFauble BryanFauble marked this pull request as ready for review August 27, 2024 23:09
@BryanFauble BryanFauble requested a review from linglp August 27, 2024 23:09
Copy link
Contributor

@linglp linglp left a comment

Choose a reason for hiding this comment

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

Thanks! I am going to pre-approve too!

@linglp
Copy link
Contributor

linglp commented Aug 28, 2024

@BryanFauble I also mentioned the Jira ticket number in the PR description so that this PR is automatically links to the ticket.

@thomasyu888
Copy link
Member

thomasyu888 commented Aug 28, 2024

@BryanFauble I also mentioned the Jira ticket number in the PR description so that this PR is automatically links to the ticket.

@linglp DPE adds the Jira ticket at the Github PR name [FDS-2294] Run schematic_api tests and that adds the Jira linkage and it its also in the branch name

@linglp
Copy link
Contributor

linglp commented Aug 28, 2024

@BryanFauble I also mentioned the Jira ticket number in the PR description so that this PR is automatically links to the ticket.

@linglp DPE adds the Jira ticket at the Github PR name [FDS-2294] Run schematic_api tests and that adds the Jira linkage and it its also in the branch name

thanks for clarifying. I just noticed that

linglp and others added 2 commits August 28, 2024 09:07
* add unit test of get_ca_alias

* add test, remove print statement
* Adding code coverage for sonarcloud
@BryanFauble BryanFauble merged commit def3bde into develop Aug 28, 2024
5 checks passed
@BryanFauble BryanFauble deleted the fds-2294-enable-testing branch August 28, 2024 16:13
Copy link

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.

3 participants