heartbeat: add test that pings the frontend url #2039
Conversation
DeltaMichael
force-pushed
the
person/mdilyan/heartbeat-ping-frontend
branch
2 times, most recently
from
66f6dde
to
dc50362
Compare
|
It seems confusing, that the proposal for vdk-heartbeat is to be configurable towards Control Service tests only. At the same time, having the Frontend tests always running. I am not convinced this is the entire VDK heartbeat API change needed. The reason is, that the Control Service is a hard prerequisite as-is while the Frontend service can be used in a variety of ways (not necessarily using the service from the helm chart, for example like a NPM library). This is visualised with the Data Framework components https://github.com/vmware/versatile-data-kit#what-vdk-can-do May you elaborate on the configurational changes you propose, before and after -> covering both services? And why is skipping tests needed for? |
DeltaMichael
force-pushed
the
person/mdilyan/heartbeat-ping-frontend
branch
from
da4fd3c
to
3909d86
Compare
DeltaMichael
force-pushed
the
person/mdilyan/heartbeat-ping-frontend
branch
from
3909d86
to
190b51d
Compare
DeltaMichael
force-pushed
the
person/mdilyan/heartbeat-ping-frontend
branch
from
190b51d
to
68a89b9
Compare
Why The frontend is packaged in the quickstart-vdk helm chart. Adding a smoke test to check if the frontend starts correctly provides an additional layer of release confidence. What Split test templates into HeartbeatTest inherits from HeartbeatBaseTest HeartbeatBaseTest has the setup, execute_test and clean_up methods. Its run_test method sets up, executes and cleans up on errors HeartBeatTest does the same, but in addtion, it - Creates a Data Job using VDK CLI for managing jobs - Deploys the data job using the same Versatile Data Kit SDK - Run database test to verify the job works correctly in cloud runtime. - Deletes the Data Job using VDK CLI Test templates now have run_test methods. Run_test methods contain the logic, which was previously in heartbeat.py for HeartbeatTest and plain logic, which does not include data job creation for HeartbeatBaseTest Add a test that does a simple get request to the frontend URL and expects a success status How was this tested Tested locally by running the quickstart-vdk and running the trino test against a local instance of trinodb Tested locally by running the ping frontend test against a local instance of the frontend Ran tests included in heartbeat-vdk CI What kind of change is this Feature, breaking Signed-off-by: Dilyan Marinov <mdilyan@vmware.com>
DeltaMichael
force-pushed
the
person/mdilyan/heartbeat-ping-frontend
branch
from
68a89b9
to
631230c
Compare
| """ | ||
| After the test is finished it cleans up all resources used by the test. | ||
| """ | ||
| pass |
There was a problem hiding this comment.
nit: Is it required to override clean_up or is it optional? Should be optional in case needed, so no need to abstractmethod
| """ | ||
| Setup the test and all that is necessary | ||
| """ | ||
| pass |
There was a problem hiding this comment.
nit: setup would also be optional in case needed;
| from vdk.internal.heartbeat.tracing import LogDecorator | ||
|
|
||
| log = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class HeartbeatTest(ABC): | ||
| class HeartbeatTest(HeartbeatBaseTest): |
There was a problem hiding this comment.
nit: Why is this class splitted from the superclass definition? I like a minimalistic approach of having the abstractions grouped by purpose commonality
| - Creates a Data Job using VDK CLI for managing jobs | ||
| - Deploys the data job using the same Versatile Data Kit SDK | ||
| - Run database test to verify the job works correctly in cloud runtime. | ||
| - Deletes the Data Job using VDK CLI | ||
| """ | ||
|
|
||
| def __init__(self, config: Config): | ||
| self.config = config |
There was a problem hiding this comment.
Call to __init__ of superclass is missing
Note: I'll add the test to the CI in a separate PR
Why
The frontend is packaged in the quickstart-vdk helm chart. Adding a
smoke test to check if the frontend starts correctly provides
an additional layer of release confidence.
What
Split test templates into HeartbeatTest inherits from HeartbeatBaseTest
HeartbeatBaseTest has the setup, execute_test and clean_up methods.
Its run_test method sets up, executes and cleans up on errors
HeartBeatTest does the same, but in addtion, it
Test templates now have run_test methods. Run_test methods contain the logic, which
was previously in heartbeat.py for HeartbeatTest and plain logic, which
does not include data job creation for HeartbeatBaseTest
Add a test that does a simple get request to the
frontend URL and expects a success status
How was this tested
Tested locally by running the quickstart-vdk and running the
trino test against a local instance of trinodb
Tested locally by running the ping frontend test against
a local instance of the frontend
Ran tests included in heartbeat-vdk
CI
What kind of change is this
Feature, breaking