Migrate E2E features #15931

ofek · 2023-09-29T03:53:47Z

Notes

Output from environments now displays in real time for better insight into what's happening
- This is mostly achieved by changing the communication mechanism to use a file rather than stdout
The env test command no longer requires the --new-env flag as it is now intelligent enough to spin up or down environments based on the current state
The new command env agent INTEGRATION ENVIRONMENT [ARGS] allows for passing arbitrary arguments directly to the agent. The check command is special in that you can omit its integration argument for ease-of-use.
- The env check command is now hidden and is a mere wrapper around the new env agent command. This will be removed in future.
The new command group env config allows for more featureful management of environment config files
The env ls command has been renamed to env show with prettier output and more granularity
The env prune command has been removed as the tooling automatically tears down and removes state upon errors

Next steps

Release datadog_checks_dev so consumers can use the updated pytest plugin
Update the minimum version of the dev package required by ddev
Release ddev

Future

Update invocations of the E2E test fixture to remove legacy options
Remove backward compatibility logic from ddev
Test utilities like docker_run and conditions that have to wait should be changed to show output

Example

codecov · 2023-09-29T03:56:43Z

Codecov Report

Merging #15931 (c8f2428) into master (87e3a45) will increase coverage by 1.31%.
Report is 1 commits behind head on master.
The diff coverage is 50.00%.

Flag	Coverage Δ
activemq	`?`
aerospike	`87.17% <ø> (+0.32%)`	⬆️
airflow	`89.57% <ø> (ø)`
cassandra	`?`
cassandra_nodetool	`93.16% <ø> (ø)`
ceph	`91.07% <ø> (ø)`
clickhouse	`95.49% <ø> (ø)`
confluent_platform	`?`
couch	`95.43% <ø> (+0.24%)`	⬆️
couchbase	`84.28% <ø> (ø)`
datadog_checks_base	`89.67% <ø> (+9.14%)`	⬆️
datadog_checks_dev	`79.93% <100.00%> (+0.03%)`	⬆️
elastic	`93.68% <ø> (+0.32%)`	⬆️
gitlab	`92.10% <ø> (+1.21%)`	⬆️
glusterfs	`80.09% <ø> (+0.90%)`	⬆️
haproxy	`95.13% <ø> (+0.16%)`	⬆️
hive	`?`
hivemq	`?`
hudi	`?`
ibm_db2	`95.35% <ø> (ø)`
ibm_was	`96.08% <ø> (ø)`
ignite	`?`
jboss_wildfly	`?`
kafka_consumer	`93.23% <ø> (-0.30%)`	⬇️
kong	`87.56% <ø> (ø)`
marklogic	`96.51% <ø> (ø)`
mysql	`87.42% <ø> (ø)`
postgres	`91.83% <ø> (+0.04%)`	⬆️
presto	`?`
rabbitmq	`96.04% <ø> (ø)`
sap_hana	`91.64% <ø> (+0.26%)`	⬆️
scylla	`99.49% <ø> (ø)`
snmp	`47.16% <0.00%> (+0.03%)`	⬆️
solr	`?`
sonarqube	`98.24% <ø> (ø)`
sqlserver	`86.48% <ø> (-1.00%)`	⬇️
ssh_check	`91.58% <ø> (ø)`
tcp_check	`90.23% <ø> (-2.70%)`	⬇️
tomcat	`?`
vault	`95.53% <ø> (+0.57%)`	⬆️
vertica	`98.34% <ø> (ø)`
voltdb	`96.85% <ø> (ø)`
vsphere	`95.79% <ø> (+0.06%)`	⬆️
win32_event_log	`86.98% <ø> (+0.66%)`	⬆️
zk	`82.62% <ø> (+1.78%)`	⬆️

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

github-actions · 2023-09-29T06:34:08Z

Test Results

    528 files     528 suites 6h 14m 35s ⏱️
  6 121 tests   5 604 ✔️   496 💤 21 ❌
24 779 runs 20 916 ✔️ 3 824 💤 39 ❌

For more details on these failures, see this check.

Results for commit c8f2428.

♻️ This comment has been updated with latest results.

FlorentClarret

I started to read but I'm not completely done yet, I'll continue reading it tomorrow morning. In the meantime, I left some (probably stupid) comments and questions!

datadog_checks_dev/datadog_checks/dev/plugin/pytest.py

ddev/pyproject.toml

FlorentClarret · 2023-10-09T14:18:44Z

ddev/src/ddev/cli/env/__init__.py

+from ddev.cli.env.agent import agent
+from ddev.cli.env.check import check
+from ddev.cli.env.config import config
+from ddev.cli.env.reload import reload_command
+from ddev.cli.env.shell import shell
+from ddev.cli.env.show import show
+from ddev.cli.env.start import start
+from ddev.cli.env.stop import stop
+from ddev.cli.env.test import test_command


FlorentClarret · 2023-10-09T14:24:23Z

ddev/src/ddev/cli/env/config.py

+    from ddev.e2e.config import EnvDataStorage
+
+    integration = app.repo.integrations.get(intg_name)
+    env_data = EnvDataStorage(app.data_dir).get(integration.name, environment)
+
+    if not env_data.exists():
+        app.abort(f'Environment `{environment}` for integration `{integration.name}` is not running')


It looks like this section is copy/pasted a few times, could we find a way to avoid this repetition?

We could, but what that would look like would be a utility function that takes (app, env_data, environment, integration_name). I'm not sure that is worth it; what do you think?

That or a decorator would work yes. Not sure that's worth it though you're right.

FlorentClarret · 2023-10-09T14:31:21Z

ddev/src/ddev/e2e/agent/docker.py

+
+        for command in (
+            ['docker', 'stop', '-t', '0', self._container_name],
+            ['docker', 'rm', self._container_name],


We do not docker run --rm in the start function for debugging purpose right?

Yes precisely! I just added a comment explaining that

FlorentClarret

Thanks for all the answers so far! Left some more comments. I probably missed some stuff and will review once more a bit later today or tomorrow morning, sorry.

FlorentClarret · 2023-10-10T12:53:41Z

ddev/src/ddev/cli/env/config.py

+    from ddev.e2e.config import EnvDataStorage
+
+    integration = app.repo.integrations.get(intg_name)
+    env_data = EnvDataStorage(app.data_dir).get(integration.name, environment)
+
+    if not env_data.exists():
+        app.abort(f'Environment `{environment}` for integration `{integration.name}` is not running')


That or a decorator would work yes. Not sure that's worth it though you're right.

FlorentClarret · 2023-10-10T12:56:52Z

ddev/src/ddev/cli/env/show.py

+        agent = get_agent_interface(agent_type)(app.platform, integration, environment, metadata, env_data.config_file)
+
+        app.display_pair('Agent type', agent_type)
+        app.display_pair('Agent ID', agent.get_id())


I would have extracted the three different blocks into their own functions but feel free to keep it as it is

FlorentClarret · 2023-10-10T12:57:56Z

ddev/src/ddev/cli/env/show.py

+                metadata = env_data.read_metadata()
+
+                columns['Name'][i] = name
+                columns['Agent type'][i] = metadata.get('agent_type', DEFAULT_AGENT_TYPE)


Should we use a constant instead of the hardcoded 'agent_type' everywhere?

Good idea, done!

FlorentClarret · 2023-10-10T13:44:27Z

ddev/src/ddev/cli/env/test.py

+        env_data = storage.get(integration.name, env_name)
+        metadata = env_data.read_metadata()
+        try:
+            with EnvVars(metadata.get('e2e_env_vars', {})):


We could also use a constant here instead of the hardcoded e2e_env_vars (similar to my other suggestion for agent_type)

FlorentClarret · 2023-10-10T13:48:35Z

ddev/src/ddev/e2e/agent/docker.py

+                    remaining = ':'.join(parts[2:])
+                    volumes[i] = f'/{vm_file}:{remaining}'
+
+        process = self._run_command(['docker', 'pull', agent_build])


It may not be related to this PR directly but what if I want to use a docker image built locally? Would the pull command fail and the command just stops? (asking because it happened to me a few times and I just ended up commenting this line)

I added an environment variable DDEV_E2E_DOCKER_NO_PULL

FlorentClarret

I left one last nit comment. It looks good to me, I tried all the commands locally and it worked well. I really like the fact that we also have the docker related info (pulling and co) so we know what's going on exactly! Really good job there 🏅

I might have missed some stuff (sorry, the PR is quite big) but I'll approve once the other comments are resolved. Thanks again for the PR!

FlorentClarret · 2023-10-11T06:53:23Z

ddev/src/ddev/cli/env/reload.py

+    agent = get_agent_interface(agent_type)(app.platform, integration, environment, metadata, env_data.config_file)
+
+    try:
+        agent.restart()


nit: I think it would be nice to have a log message to confirm that the file XYZ has been successfully reloaded after the restart call

* Migrate E2E features * address review * address review ae3fdee

ofek requested review from a team as code owners September 29, 2023 03:53

ghost added documentation dev_package ddev labels Sep 29, 2023

ofek force-pushed the ofek/e2e branch 7 times, most recently from f7cde3d to 79955e6 Compare September 29, 2023 05:26

ofek force-pushed the ofek/e2e branch 6 times, most recently from 92fb9d6 to c107823 Compare September 29, 2023 16:53

aliciascott previously approved these changes Sep 29, 2023

View reviewed changes

ofek dismissed aliciascott’s stale review via dcff4b7 September 29, 2023 18:28

ofek force-pushed the ofek/e2e branch 8 times, most recently from abc791a to eddf6eb Compare September 30, 2023 22:17

ofek force-pushed the ofek/e2e branch 14 times, most recently from 375aace to 559add4 Compare October 6, 2023 20:20

FlorentClarret reviewed Oct 9, 2023

View reviewed changes

ofek force-pushed the ofek/e2e branch from 7878919 to afae9b7 Compare October 9, 2023 14:57

FlorentClarret reviewed Oct 10, 2023

View reviewed changes

FlorentClarret reviewed Oct 11, 2023

View reviewed changes

ofek force-pushed the ofek/e2e branch from ba632ec to 004c029 Compare October 11, 2023 20:53

FlorentClarret previously approved these changes Oct 12, 2023

View reviewed changes

ofek added 3 commits October 12, 2023 10:28

Migrate E2E features

2f68343

address review

782259a

address review

c8f2428

ofek dismissed FlorentClarret’s stale review via c8f2428 October 12, 2023 14:28

ofek force-pushed the ofek/e2e branch from 004c029 to c8f2428 Compare October 12, 2023 14:28

FlorentClarret approved these changes Oct 12, 2023

View reviewed changes

ofek merged commit ae3fdee into master Oct 12, 2023

ofek deleted the ofek/e2e branch October 12, 2023 15:35

github-actions bot pushed a commit that referenced this pull request Oct 12, 2023

Migrate E2E features (#15931)

1d5ea3f

* Migrate E2E features * address review * address review ae3fdee

FlorentClarret mentioned this pull request Oct 20, 2023

Do not modify a custom agent build name provided by the user when running the e2e environments #16052

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate E2E features #15931

Migrate E2E features #15931

ofek commented Sep 29, 2023 •

edited

Loading

codecov bot commented Sep 29, 2023 •

edited

Loading

github-actions bot commented Sep 29, 2023 •

edited

Loading

FlorentClarret left a comment

FlorentClarret Oct 9, 2023

FlorentClarret Oct 9, 2023

ofek Oct 9, 2023

FlorentClarret Oct 10, 2023

FlorentClarret Oct 9, 2023

ofek Oct 9, 2023

FlorentClarret left a comment

FlorentClarret Oct 10, 2023

FlorentClarret Oct 10, 2023

FlorentClarret Oct 10, 2023

ofek Oct 11, 2023

FlorentClarret Oct 10, 2023

ofek Oct 11, 2023

FlorentClarret Oct 10, 2023

ofek Oct 11, 2023

FlorentClarret left a comment

FlorentClarret Oct 11, 2023

ofek Oct 11, 2023

Migrate E2E features #15931

Migrate E2E features #15931

Conversation

ofek commented Sep 29, 2023 • edited Loading

Notes

Next steps

Future

Example

codecov bot commented Sep 29, 2023 • edited Loading

Codecov Report

github-actions bot commented Sep 29, 2023 • edited Loading

Test Results

FlorentClarret left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FlorentClarret left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FlorentClarret left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofek commented Sep 29, 2023 •

edited

Loading

codecov bot commented Sep 29, 2023 •

edited

Loading

github-actions bot commented Sep 29, 2023 •

edited

Loading