[IO-1556] Create and push E2E test #661

Nathanjp91 · 2023-09-15T08:48:11Z

Problem

No E2E tests for darwin cli exists

Solution

Add in E2E tests and fixes to get tests running

Changelog

Testing added for create and basic push tests via cli

linear · 2023-09-15T08:48:14Z

IO-1556 e2e test: push dataset

Nathanjp91 · 2023-09-18T09:58:21Z

darwin/cli.py

        f._error("Darwin seems unreachable, please try again in a minute or contact support.")
    except GracefulExit as e:
-        f.error(e.message)
+        f._error(e.message)


Not sure how this made it in but may as well stay

Yeah, that is a bit weird, but nvm as you say.

owencjones

Good stuff man, sorry I wasn't about to help out more tail end of the week.

owencjones · 2023-09-18T09:58:30Z

.github/workflows/tests.yml

          pip install pytest pytest-describe
      - name: Run Tests
        run: python -m pytest -W ignore::DeprecationWarning
+  e2e:


I'll need to transplant these in the new refactor of the GHAs, but that's my problem for not getting them in first.

owencjones · 2023-09-18T10:00:09Z

.github/workflows/tests.yml

+          pip install --upgrade setuptools
+          pip install --editable ".[test,ml,medical,dev]"
+          pip install pytest pytest-describe
+      - name: Run Tests


This could be added as a step under the existing installs from the tests job, but I'd say in this instance, I've got a load of pending changes to this area, including this, so I can incorporate it.

owencjones · 2023-09-18T10:01:10Z

e2e_tests/conftest.py

+    # pytest.datasets = datasets
+    setattr(pytest, "datasets", datasets)
+    # Set the environment variables for running CLI arguments
+    os.environ["DARWIN_BASE_URL"] = server


owencjones · 2023-09-18T10:06:02Z

e2e_tests/helpers.py

        )
-
-    return result.returncode, result.stdout.decode("utf-8"), result.stderr.decode("utf-8")
+    try:


Good work around, but I really hate this for the stdlib inconsistencies!

I guess it makes sense given it's just a byte string, but still

owencjones · 2023-09-18T10:08:37Z

e2e_tests/test_darwin.py

@@ -0,0 +1,68 @@
+import re


Nice first test. Good to see it starting to come together.

brain-geek · 2023-09-19T14:13:51Z

.github/workflows/tests.yml

+          E2E_ENVIRONMENT: ${{ secrets.E2E_ENVIRONMENT }}
+          E2E_TEAM: ${{ secrets.E2E_TEAM }}
+
+


too much newlines

brain-geek · 2023-09-19T14:14:00Z

.github/workflows/tests.yml

          pip install pytest pytest-describe
      - name: Run Tests
        run: python -m pytest -W ignore::DeprecationWarning
+  e2e:


What do you think about moving this to separate file?

brain-geek · 2023-09-19T14:14:58Z

e2e_tests/conftest.py

+    setattr(pytest, "datasets", datasets)
+    # Set the environment variables for running CLI arguments
+    environ["DARWIN_BASE_URL"] = server
+    environ["DARWIN_TEAM"] = team_slug


Generally, after setting ENV's you want to revert them, as this is change to global state.

It's slightly misleading - it looks like a change to global state, but posix env vars, which these will be in most cases, are in one of two scopes, global or local - os.environ is changing the local (process) scope, at least in a posix-like system.

In windows, it might set it globally.

Obv, unsetting these wouldn't hurt, just adding context.

brain-geek · 2023-09-19T14:15:05Z

e2e_tests/helpers.py

+    try:
+        return result.returncode, result.stdout.decode("utf-8"), result.stderr.decode("utf-8")
+    except:
+        return result.returncode, result.stdout.decode("cp437"), result.stderr.decode("cp437")


brain-geek · 2023-09-19T14:15:40Z

e2e_tests/test_darwin.py

+if __name__ == "__main__":
+    pytest.main(["-vv", "-s", __file__])
+
+


brain-geek · 2023-09-19T14:15:53Z

e2e_tests/test_darwin.py

+    assert local_dataset.name is not None
+
+
+


is this different amount of lines between tests intended?

brain-geek · 2023-09-19T14:16:13Z

e2e_tests/conftest.py

    api_key = environ.get("E2E_API_KEY")
    team_slug = environ.get("E2E_TEAM")

+


owencjones

Yep, like it, all looks good, and the slack integ is good blueprint for the incoming one from my branch

owencjones · 2023-09-19T16:50:39Z

.github/workflows/e2e.yml

+          {
+            "channel": "#${{ vars.SLACK_CHANNEL }}",
+            "username": "${{ vars.SLACK_USERNAME }}",
+            "text": "Nightly E2E run failed <!subteam^S04T5MWDHJ6>: https://github.com/v7labs/darwin-py/actions/runs/${{ github.run_id }}",


Fun fact: you can also email this id and send a message.

owencjones · 2023-09-19T16:51:27Z

darwin/cli.py

        f._error("Darwin seems unreachable, please try again in a minute or contact support.")
    except GracefulExit as e:
-        f.error(e.message)
+        f._error(e.message)


Yeah, that is a bit weird, but nvm as you say.

owencjones · 2023-09-19T16:55:39Z

e2e_tests/conftest.py

+    setattr(pytest, "datasets", datasets)
+    # Set the environment variables for running CLI arguments
+    environ["DARWIN_BASE_URL"] = server
+    environ["DARWIN_TEAM"] = team_slug


It's slightly misleading - it looks like a change to global state, but posix env vars, which these will be in most cases, are in one of two scopes, global or local - os.environ is changing the local (process) scope, at least in a posix-like system.

In windows, it might set it globally.

Obv, unsetting these wouldn't hurt, just adding context.

owencjones · 2023-09-19T16:56:12Z

e2e_tests/conftest.py

+    del environ["DARWIN_BASE_URL"]
+    del environ["DARWIN_TEAM"]
+    del environ["DARWIN_API_KEY"]
+


Heh, @brain-geek turns out it doesn't matter anymore anyway!

Nathanjp91 added 3 commits September 13, 2023 22:28

e2e testing create

cacc32f

test teardown refactor

87b5bd3

cleanup

be854d6

Nathanjp91 added 10 commits September 15, 2023 16:31

Merge branch 'master' into IO-1556

dd83705

e2e environment variables

a4ade3f

Merge branch 'master' into IO-1556

aa31533

e2e environment variables

96de813

e2e environment variables

d6d5ea7

testing fix for windows

ce1ae14

diagnostics

7a703ce

potential windows fix

f50e29d

potential windows fix

b56eb84

potential windows fix

0494528

Nathanjp91 commented Sep 18, 2023

View reviewed changes

cleanup

cf09f0e

owencjones approved these changes Sep 18, 2023

View reviewed changes

brain-geek reviewed Sep 19, 2023

View reviewed changes

Nathan Perkins added 11 commits September 19, 2023 15:53

e2e nightly@

7168e19

e2e nightly test

d84add1

e2e nightly test + slack

b9448d1

e2e nightly test + slack

097cf07

slack fix?

138b648

slack fix?

0f1d075

slack fix?

e427cdf

slack fix

8fabf19

cleanup

324bb31

cleanup

a41eb2a

scheduled task sanity check

a6e0319

Nathanjp91 requested a review from owencjones September 19, 2023 16:49

owencjones approved these changes Sep 19, 2023

View reviewed changes

Nathanjp91 merged commit 5485f1b into master Sep 19, 2023

		E2E_ENVIRONMENT: ${{ secrets.E2E_ENVIRONMENT }}
		E2E_TEAM: ${{ secrets.E2E_TEAM }}

		api_key = environ.get("E2E_API_KEY")
		team_slug = environ.get("E2E_TEAM")

[IO-1556] Create and push E2E test #661

[IO-1556] Create and push E2E test #661

Uh oh!

Conversation

Nathanjp91 commented Sep 15, 2023

Problem

Solution

Changelog

Uh oh!

linear bot commented Sep 15, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owencjones left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owencjones left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants