test: use pytest-forked lib to run test in forked process #5263

girishc13 · 2022-10-10T08:53:33Z

Goals:

Run tests in forked process to avoid flakiness and reusing process state for subsequent Flow's. Shutting down a Flow leaves the module space in an inconsistent state so forking multiple Flow in pytest is not so useful.

JoanFM

Apply same change to cd.yml

codecov · 2022-10-10T09:01:19Z

Codecov Report

Merging #5263 (afc59f9) into master (a8dce57) will increase coverage by 10.29%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #5263       +/-   ##
===========================================
+ Coverage   64.81%   75.11%   +10.29%     
===========================================
  Files          98      100        +2     
  Lines        6387     6433       +46     
===========================================
+ Hits         4140     4832      +692     
+ Misses       2247     1601      -646

Flag	Coverage Δ
jina	`75.11% <ø> (+10.38%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...a/orchestrate/deployments/config/docker_compose.py	`99.01% <0.00%> (-0.99%)`	⬇️
jina/resources/health_check/pod.py	`50.00% <0.00%> (ø)`
jina/resources/health_check/gateway.py	`41.17% <0.00%> (ø)`
jina/clients/base/websocket.py	`83.80% <0.00%> (+0.95%)`	⬆️
jina/clients/base/http.py	`91.89% <0.00%> (+1.35%)`	⬆️
jina/serve/networking.py	`67.53% <0.00%> (+2.81%)`	⬆️
jina/clients/base/helper.py	`81.88% <0.00%> (+3.14%)`	⬆️
jina/clients/base/grpc.py	`82.85% <0.00%> (+4.28%)`	⬆️
jina/orchestrate/flow/base.py	`90.15% <0.00%> (+4.29%)`	⬆️
jina/jaml/helper.py	`75.18% <0.00%> (+4.37%)`	⬆️
... and 39 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

…t-forked-v2

JoanFM · 2022-10-10T10:16:58Z

tests/unit/test_yamlparser.py

@@ -120,6 +120,7 @@ def test_encoder_name_env_replace():


 def test_encoder_name_dict_replace():
+    os.environ['BE_TEST_NAME'] = 'hello123'


can u please use a fixture to set the env variable and reset afterwards? this is done in many other places

tests/unit/test_yamlparser.py

girishc13 · 2022-10-10T13:29:05Z

@JoanFM I haven't added the arguments to the cd.yml yet. One thing to also notice with this change is that the CI logs don't capture the logging of each individual tests. The logs are not available because each test run in its own process. Locally we can remove the --forked argument and obtain the logs but on CI we might need to do some commits in case we want a detailed log.

JoanFM · 2022-10-10T13:47:59Z

@JoanFM I haven't added the arguments to the cd.yml yet. One thing to also notice with this change is that the CI logs don't capture the logging of each individual tests. The logs are not available because each test run in its own process. Locally we can remove the --forked argument and obtain the logs but on CI we might need to do some commits in case we want a detailed log.

if it guarantees non-flakyness, I would still pay the price, however there must be some way to solve this

girishc13 · 2022-10-10T15:58:02Z

Unfortunately the pytest-forked plugin isn't currently maintained. This open issue is causing the tests/unit/orchestrate/pods/ to fail. I'm not sure of an alternative to this test.

girishc13 · 2022-10-10T16:11:03Z

The whole test code for test_pod.py looks unstable on close inspection and logging. The same pid is being reused across all tests. I added some logging to check the pid of each SlowFakeRuntime. The first runtime Pod@41979 runs throughout all the tests so I'm not sure what the tests are doing.

pytest --suppress-no-test-exit-code -v -s --ignore-glob='tests/integration/hub_usage/dummyhub*' tests/unit/orchestrate/pods/test_pod.py | grep -e "creating"
tests/unit/orchestrate/pods/test_pod.py::test_pod_runtime_env_setting DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:04]
tests/unit/orchestrate/pods/test_pod.py::test_gateway_args[grpc-GRPCGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:04]
tests/unit/orchestrate/pods/test_pod.py::test_gateway_args[websocket-WebSocketGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:04]
tests/unit/orchestrate/pods/test_pod.py::test_gateway_args[http-HTTPGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:04]
tests/unit/orchestrate/pods/test_pod.py::test_gateway_runtimes[grpc-GRPCGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:04]
tests/unit/orchestrate/pods/test_pod.py::test_gateway_runtimes[websocket-WebSocketGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:04]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     Uvicorn running on http://0.0.0.0:52345 (Press CTRL+C to quit)
INFO:     Shutting down
INFO:     Waiting for application shutdown.
INFO:     Application shutdown complete.
tests/unit/orchestrate/pods/test_pod.py::test_gateway_runtimes[http-HTTPGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:05]
INFO:     Waiting for application startup.
INFO:     Application startup complete.
INFO:     Uvicorn running on http://0.0.0.0:50697 (Press CTRL+C to quit)
INFO:     Shutting down
INFO:     Waiting for application shutdown.
INFO:     Application shutdown complete.
tests/unit/orchestrate/pods/test_pod.py::test_non_gateway_runtimes[WorkerRuntime] DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:05]
tests/unit/orchestrate/pods/test_pod.py::test_non_gateway_runtimes[HeadRuntime] DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:05]
tests/unit/orchestrate/pods/test_pod.py::test_failing_executor DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:06]
tests/unit/orchestrate/pods/test_pod.py::test_failing_gateway_runtimes[grpc-GRPCGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:06]
tests/unit/orchestrate/pods/test_pod.py::test_failing_gateway_runtimes[websocket-WebSocketGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:06]
tests/unit/orchestrate/pods/test_pod.py::test_failing_gateway_runtimes[http-HTTPGateway] DEBUG  gateway@41979 --->creating new pod...                                                                                  [10/10/22 18:08:06]
tests/unit/orchestrate/pods/test_pod.py::test_failing_head DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:06]
tests/unit/orchestrate/pods/test_pod.py::test_close_before_start DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:06]
tests/unit/orchestrate/pods/test_pod.py::test_close_before_start_slow_enter DEBUG  Pod@41979 --->creating new pod...                                                                                      [10/10/22 18:08:06]

girishc13 · 2022-10-11T10:19:00Z

Some tests might still be flaky due to the incompatibility with the pytest-timeout feature.

JoanFM · 2022-10-11T15:45:33Z

Unfortunately the pytest-forked plugin isn't currently maintained. This open issue is causing the tests/unit/orchestrate/pods/ to fail. I'm not sure of an alternative to this test.

if it is not mantained, I would remove this

girishc13 · 2022-10-12T07:26:00Z

The pytest-forked library currently doesn't have a maintainer and the open issue with pytest-timeout prevents us form investing further time.

test: use pytst-forked lib to run test in forked process

5915e7d

girishc13 self-assigned this Oct 10, 2022

github-actions bot added size/XS area/cicd This issue/PR affects the cicd pipeline area/housekeeping This issue/PR is housekeeping area/setup This issue/PR affects setting up Jina labels Oct 10, 2022

style: fix overload and cli autocomplete

14cd2b6

JoanFM requested changes Oct 10, 2022

View reviewed changes

JoanFM closed this Oct 10, 2022

JoanFM reopened this Oct 10, 2022

github-actions bot added size/S area/core This issue/PR affects the core codebase component/resource labels Oct 10, 2022

girishc13 changed the title ~~test: use pytst-forked lib to run test in forked process~~ test: use pytest-forked lib to run test in forked process Oct 10, 2022

Girish Chandrashekar added 2 commits October 10, 2022 12:12

fix: set required env variable for test

8a9596e

Merge branch 'pytest-forked-v2' of github.com:jina-ai/jina into pytes…

63d685e

…t-forked-v2

github-actions bot added the area/testing This issue/PR affects testing label Oct 10, 2022

JoanFM requested changes Oct 10, 2022

View reviewed changes

fix: use fixture to set and del test env variables

26fcdd7

JoanFM previously approved these changes Oct 10, 2022

View reviewed changes

tests/unit/test_yamlparser.py Outdated Show resolved Hide resolved

fix: remove redundant env var in tests

4a6b21f

girishc13 dismissed JoanFM’s stale review via 4a6b21f October 10, 2022 13:27

test: use pytest-forked lib to run CD tests in forked process

e00a97b

fix: remove --forked option for k8s tests

ce1787e

fix: remove flaky pod slow enter test

20b069d

Merge branch 'master' into pytest-forked-v2

afc59f9

girishc13 closed this Oct 12, 2022

girishc13 deleted the pytest-forked-v2 branch October 12, 2022 07:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: use pytest-forked lib to run test in forked process #5263

test: use pytest-forked lib to run test in forked process #5263

girishc13 commented Oct 10, 2022 •

edited

Loading

JoanFM left a comment

codecov bot commented Oct 10, 2022 •

edited

Loading

JoanFM Oct 10, 2022

girishc13 commented Oct 10, 2022

JoanFM commented Oct 10, 2022

girishc13 commented Oct 10, 2022

girishc13 commented Oct 10, 2022 •

edited

Loading

girishc13 commented Oct 11, 2022 •

edited

Loading

JoanFM commented Oct 11, 2022

girishc13 commented Oct 12, 2022

		@@ -120,6 +120,7 @@ def test_encoder_name_env_replace():


		def test_encoder_name_dict_replace():
		os.environ['BE_TEST_NAME'] = 'hello123'

test: use pytest-forked lib to run test in forked process #5263

test: use pytest-forked lib to run test in forked process #5263

Conversation

girishc13 commented Oct 10, 2022 • edited Loading

JoanFM left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 10, 2022 • edited Loading

Codecov Report

JoanFM Oct 10, 2022

Choose a reason for hiding this comment

girishc13 commented Oct 10, 2022

JoanFM commented Oct 10, 2022

girishc13 commented Oct 10, 2022

girishc13 commented Oct 10, 2022 • edited Loading

girishc13 commented Oct 11, 2022 • edited Loading

JoanFM commented Oct 11, 2022

girishc13 commented Oct 12, 2022

girishc13 commented Oct 10, 2022 •

edited

Loading

codecov bot commented Oct 10, 2022 •

edited

Loading

girishc13 commented Oct 10, 2022 •

edited

Loading

girishc13 commented Oct 11, 2022 •

edited

Loading