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.

Sign up for GitHub

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

Jump to bottom

test(dp): Retain DB state in orc8r integration tests #12607

Merged

xbend merged 1 commit into magma:master from openEPC:dp_tests_improvements

May 2, 2022

Contributor

xbend commented Apr 29, 2022

Summary

Do not clear/initialize DB in orc8r integration tests setup/teardown
Clear/initialize DB in orc8r integration testcase classes
Use randmized CBSD serial numbers per orc8r integration test to meet unique constraint on that field in the (now retained between tests) DB

Additional Information

This change is backwards-breaking

xbend requested review from a team and taaraora

April 29, 2022 17:00

pull-request-size bot added the size/L label

Contributor

github-actions bot commented Apr 29, 2022

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

Contributor

github-actions bot commented Apr 29, 2022 •

edited

Oops! Looks like you failed the DCO check. Be sure to sign all your commits.

Howto

♻️ Updated: ✅ The check is passing the DCO check after the last commit.

xbend force-pushed the dp_tests_improvements branch from 7346ea9 to 336de04 Compare

April 29, 2022 17:32

jkmar reviewed

View reviewed changes

dp/cloud/python/magma/test_runner/tests/test_dp_with_orc8r.py Outdated

@@ @@ -389,26 +420,31 @@ def send_request_to_backend( @@
                   method: str, url_suffix: str, params: Optional[Dict[str, Any]] = None,
                   json: Optional[Dict[str, Any]] = None,
               ) -> requests.Response:
-                  return requests.request(

Contributor

jkmar May 2, 2022

there is no need for this change (I guess it was just used for debugging)

Contributor Author

xbend May 2, 2022

Reverted.

dp/cloud/python/magma/test_runner/tests/test_dp_with_orc8r.py

@@ @@ -378,7 +409,7 @@ def wait_for_elastic_to_start() -> None: @@
               def when_elastic_indexes_data():
                   # TODO use retrying instead
-                  sleep(5)
+                  sleep(15)

Contributor

jkmar May 2, 2022

This is painful, can we create a ticket for using retries for elastic?

Contributor Author

xbend May 2, 2022

We have it: https://app.zenhub.com/workspaces/magma--domain-proxy-development-6064cb9d84ca0600135548c9/issues/magma/domain-proxy/364

dp/cloud/python/magma/test_runner/tests/test_dp_with_orc8r.py Outdated

@@ @@ -19,6 +19,8 @@ @@
               from threading import Event, Thread
               from time import sleep
               from typing import Any, Dict, List, Optional
+              from unittest import TestCase

Contributor

jkmar May 2, 2022

looks unused

Contributor Author

xbend May 2, 2022

Removed.

dp/cloud/python/magma/test_runner/tests/test_dp_with_orc8r.py Outdated

@@ @@ -245,6 +260,17 @@ def when_cbsds_are_fetched( @@
                       self.assertEqual(len(cbsds), expected_cbsds_num)
                       return cbsds
+                  def when_cbsd_is_fetched(self, serial_number: str = None) -> Dict[str, Any]:
+                      params = {}
+                      params['serial_number'] = serial_number

Contributor

jkmar May 2, 2022

why not just params = {'serial_number': serial_number}?

Contributor Author

xbend May 2, 2022

Redone as suggested.

dp/cloud/python/magma/test_runner/tests/test_dp_with_orc8r.py Outdated

-                  def then_cbsd_is_deleted(self):
-                      r = send_request_to_backend('get', 'cbsds')
+                  def then_cbsd_is_deleted(self, serial_number: str):
+                      params = {}

Contributor

jkmar May 2, 2022

there is a lot of duplication between this method and when_cbsd_is_fetched method, can common part be extracted?

Contributor Author

xbend May 2, 2022

Added private check method.

xbend force-pushed the dp_tests_improvements branch from 336de04 to 6bd6f88 Compare

May 2, 2022 12:00


          test(dp): Retain DB state in orc8r integration tests

84e98dc

* Do not clear/initialize DB in orc8r integration tests setup/teardown
* Clear/initialize DB in orc8r integration testcase classes
* Use randmized CBSD serial numbers per orc8r integration test
to meet unique constraint on that field in the (now retained between tests) DB

Signed-off-by: Artur Dębski <artur.debski@freedomfi.com>

xbend force-pushed the dp_tests_improvements branch from 6bd6f88 to 84e98dc Compare

May 2, 2022 12:42

jkmar approved these changes

View reviewed changes

xbend removed the request for review from taaraora

May 2, 2022 21:17

xbend merged commit 45385b1 into magma:master

xbend deleted the dp_tests_improvements branch

May 2, 2022 21:21

emakeev pushed a commit to emakeev/magma that referenced this pull request


          test(dp): Retain DB state in orc8r integration tests (magma#12607)

28454c6

* Do not clear/initialize DB in orc8r integration tests setup/teardown
* Clear/initialize DB in orc8r integration testcase classes
* Use randmized CBSD serial numbers per orc8r integration test
to meet unique constraint on that field in the (now retained between tests) DB

Signed-off-by: Artur Dębski <artur.debski@freedomfi.com>

Co-authored-by: Artur Dębski <artur.debski@freedomfi.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment