Killing the Discovery Service started by the Test Environment

[MounikaBattu17]

• This contribution adheres to CONTRIBUTING.md.

What does this Pull Request accomplish?

Acceptance tests (test_measurement_service.py, test_streaming_data_measurement.py, test_yield_vs_return.py) starts the discovery service for running their tests but doesn't terminate/kill it. This causes a Resource Warning while running all tests.
Approach is to start the Discovery service independently and scope it to whole test session.

1. Created a Context Manager DiscoveryServiceProcess in tests/utilities/discovery_service_process.py.
   • __init__ is responsible for starting the discovery service and store the instance in self._proc.
   • __exit__ is responsible for killing the active Discovery service instance.
2. Added a pytest fixture discovery_service_process that is responsible for
   • skipping tests for non-windows and measurement link registration file not found.
   • yielding the proc and scoped it to session for windows platform.
3. Called the discovery_service_process fixture in measurement_service fixture in each acceptance test files.

Why should this Pull Request be merged?

1. Starts and Closes Discovery Service instances.
2. Fixes Resource Warning while running tests.

What testing has been done?

1. Ran poetry run pytest, Observed there is no Resource Warning.
2. Observed Discovery Service is started once and closed at the end of the test session.
3. Verified acceptance tests are skipped if platform is non windows or measurement link registration file not available.

[github-actions]

Test Results

     12 files  ±0       12 suites  ±0   1m 58s ⏱️ - 16m 28s
   180 tests ±0     155 ✔️  -   25    25 💤 +  25  0 ❌ ±0 
2 148 runs  ±0  1 848 ✔️  - 300  300 💤 +300  0 ❌ ±0 

Results for commit 63dc634. ± Comparison against base commit 4c2672a.

♻️ This comment has been updated with latest results.

[dixonjoel]

Please update the PR description to match the implementation since you're not using the IsInTestState global now.

[bkeryan]

@MounikaBattu17 Did #344 not fix the ResourceWarning?

[MounikaBattu17]

@MounikaBattu17 Did #344 not fix the ResourceWarning?

Yes, It fixed the resource warning. The ResourceWarning doesn't come if I run a measurement.

[bkeryan]

This causes a Resource Warning while running all tests.

If the resource warning was already fixed, why does the description of this PR talk about the resource warning?

[MounikaBattu17]

This causes a Resource Warning while running all tests.

If the resource warning was already fixed, why does the description of this PR talk about the resource warning?

The resource warning which I'm referring is from the tests. Earlier/ without this PR changes, there was a resource warning after the test run.

======================================================== warnings summary ========================================================= 
tests/unit/test_discovery_client.py::test___get_discovery_service_address___start_service_jit___returns_expected_value
  C:\Users\battu.mounika\AppData\Local\Programs\Python\Python38\lib\subprocess.py:946: ResourceWarning: subprocess 21300 is still running
    _warn("subprocess %s is still running" % self.pid,
  Enable tracemalloc to get traceback where the object was allocated.
  See https://docs.pytest.org/en/stable/how-to/capture-warnings.html#resource-warnings for more info.

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================================== 179 passed, 1 warning in 148.70s (0:02:28) ============================================

[dixonjoel]

All tests pass locally for me and don't get skipped when I have the DS installed.

[MounikaBattu17]

This PR has got necessary approval. Hence merging.