Add integration test that starts blender example in goth #281

azawlocki · 2021-03-05T15:08:00Z

This PR adds a test case that runs our default blender example (examples/blender/blender.py) in a 2-provider test network started by goth.

The test serves mainly as a sanity check and an illustrative example of how to create more test cases using goth.

To run the test case on your local machine, do this in the root directory of the yapapi project:

$ poetry install -E integration-tests
$ pytest -svx  tests/goth/test_run_blender.py

Few basic properties are checked by monitoring output of the requestor script:

Proposals are obtained from the market
Agreement is negotiated with at least one provider
All tasks are sent and computed
For every agreement an invoice is accepted
The executor finally shuts down
The requestor script does not log any message with level ERROR

filipgolem · 2021-03-10T17:23:18Z

tests/goth/assets/goth-config.yml

@@ -0,0 +1,62 @@
+# Note: values of keys denoting paths are resolved relative to the directory
+# in which this file is located.
+# The tokens `~` and `~user` are also replaced by the corresponding users's


Suggested change

# The tokens `~` and `~user` are also replaced by the corresponding users's

# The tokens `~` and `~user` are also replaced by user's

This file is now (as of f16cc5a) generated by goth in a separate workflow step.

kmazurek

Looks good, left some minor comments. I am somewhat worried about the maintainability of goth assets, but that's a separate discussion.

kmazurek · 2021-03-26T16:52:50Z

.github/workflows/integration.yml

+      # - <your-branch>    # put your branch name here to test it @ GH Actions
+  pull_request:
+    branches:
+      - master


The workflow tests.yml includes release branches (i.e. - b0.*) on pushes and pull requests. Perhaps they should be added here too?

added them in 0d80b77

kmazurek · 2021-03-26T17:01:08Z

tests/goth/assets/docker/docker-compose.yml

+            - "8545:8545"
+
+    zksync:
+        image: docker.pkg.github.com/golemfactory/yagna-zksync/yagna-zksync-mock:7a25ed913d4a


Suggested change

image: docker.pkg.github.com/golemfactory/yagna-zksync/yagna-zksync-mock:7a25ed913d4a

image: docker.pkg.github.com/golemfactory/yagna-zksync/yagna-zksync-mock:f6d0cf02f6bc

I just realised updating these image hashes is going to be troublesome once tests are split out into project repos. :x

kmazurek · 2021-03-26T17:07:04Z

tests/goth/assets/goth-config.yml

+    class: "goth.runner.probe.RequestorProbe"
+
+  - name: "VM-Wasm-Provider"
+    class: "goth.runner.provider.ProviderProbeWithLogSteps"


Suggested change

class: "goth.runner.provider.ProviderProbeWithLogSteps"

class: "goth.runner.provider.ProviderProbe"

Once golemfactory/goth#454 is merged. I'm getting more and more skeptical about having these assets duplicated across repos. :(

Good point, I've removed the asset files from the repository and instead added a command that creates them every time the tests are run -- see f16cc5a

filipgolem · 2021-03-29T07:59:56Z

.github/workflows/integration.yml

+      # - <your-branch>    # put your branch name here to test it @ GH Actions
+  pull_request:
+    branches:
+      - master


"On pull request" action for yajsapi unit tests looks like this:

Suggested change

- master

- master

- b0.*

added in 0d80b77

filipgolem · 2021-03-29T08:02:02Z

pyproject.toml

@@ -37,10 +37,11 @@ toml = "^0.10.1"
 srvresolver = "^0.3.5"
 colorama = "^0.4.4"

+goth = {git = "https://github.com/golemfactory/goth.git", branch = "master", optional = true, python = "^3.8.0"}


I think that there should be a comment explaining why goth was added as an optional dependency, not a dev dependency.

@azawlocki:

I couldn't make it work with goth as dev-dependency, as goth requires Python >= 3.8, which conflicts with Python >= 3.6 for the whole project. So goth is now installed with optional = "true" and python = "^3.8.0".
Perhaps it can be moved to dev-dependencies (it would be an optional dev-dependency then, I'm not sure if that would work though).

a comment added in 0d80b77

pyproject.toml

shadeofblue · 2021-03-29T13:58:51Z

.github/workflows/integration.yml

+          poetry install -E integration-tests
+
+      - name: Log in to GitHub Docker repository
+        run: echo ${{ secrets.GITHUB_TOKEN }} | docker login docker.pkg.github.com -u ${{github.actor}} --password-stdin


do we still need this log-in given that goth repo is now public?

We still need this for two reasons:

Pointing the Docker daemon to GitHub's package registry (by default it only uses hub.docker.com).

Basic authentication, I believe this is similar to using GitHub's REST API, i.e. you still need to provide a personal token even when accessing public endpoints, presumably to handle stuff like rate limiting.

shadeofblue · 2021-03-29T14:00:36Z

pyproject.toml

@@ -1,6 +1,6 @@
 [build-system]
-requires = ["poetry"]
-build-backend = "poetry.masonry.api"
+requires = ["poetry_core>=1.0.0"]


@azawlocki just out of curiosity - where do thse changes come from?

From the remarks in https://python-poetry.org/docs/pyproject/#poetry-and-pep-517 (I was reading that file looking for a way to add goth as an extra/optional dependency).

shadeofblue · 2021-03-29T14:01:36Z

pyproject.toml


 [tool.poetry.extras]
 cli = ['fire', 'rich']
-
+integration-tests = ['goth', 'pytest', 'pytest-asyncio']


shadeofblue

…cies

Co-authored-by: filipgolem <44880692+filipgolem@users.noreply.github.com> Co-authored-by: Kuba Mazurek <jakub.mazurek@golem.network>

azawlocki marked this pull request as draft March 5, 2021 19:19

azawlocki force-pushed the integration-tests branch 15 times, most recently from e1cd13f to 475ab70 Compare March 10, 2021 16:50

filipgolem reviewed Mar 10, 2021

View reviewed changes

azawlocki force-pushed the integration-tests branch 2 times, most recently from 556cb07 to c03d5bb Compare March 11, 2021 08:02

azawlocki mentioned this pull request Mar 11, 2021

Changes required to use goth in CI pipeline for yapapi golemfactory/goth#438

Merged

azawlocki force-pushed the integration-tests branch from 3efba97 to 22a121b Compare March 23, 2021 09:46

azawlocki marked this pull request as ready for review March 25, 2021 09:32

azawlocki changed the title ~~[WIP] Add integration test that starts blender example in goth~~ Add integration test that starts blender example in goth Mar 25, 2021

azawlocki force-pushed the integration-tests branch from 0257794 to 3a222c0 Compare March 25, 2021 09:44

azawlocki requested review from shadeofblue, kmazurek and a team March 25, 2021 10:00

kmazurek approved these changes Mar 26, 2021

View reviewed changes

filipgolem reviewed Mar 29, 2021

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

shadeofblue reviewed Mar 29, 2021

View reviewed changes

filipgolem approved these changes Mar 29, 2021

View reviewed changes

shadeofblue reviewed Mar 29, 2021

View reviewed changes

pyproject.toml

[tool.poetry.extras]

cli = ['fire', 'rich']

integration-tests = ['goth', 'pytest', 'pytest-asyncio']

Copy link

Collaborator

shadeofblue Mar 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

shadeofblue approved these changes Mar 29, 2021

View reviewed changes

azawlocki and others added 8 commits March 30, 2021 12:14

Add integration test that starts blender example in goth

dd1169d

Add GH action for running integration tests

8bcddd4

Move goth-related fixtures to tests/goth/conftest.py

522fea3

Set Python version required for goth in pyproject.toml

1558517

Add assertions for blender.py output; update goth version in dependen…

b8c3cc5

…cies

Remove poetry virtual env in the last step of integration.yml workflow

325960d

Generate default goth assets before running tests

03a79a7

Code review: enable tests for PR's to b0.*; comments in pyproject.toml

dbe0f95

Co-authored-by: filipgolem <44880692+filipgolem@users.noreply.github.com> Co-authored-by: Kuba Mazurek <jakub.mazurek@golem.network>

azawlocki force-pushed the integration-tests branch from ca8531d to dbe0f95 Compare March 30, 2021 10:16

azawlocki merged commit 404bbb0 into master Mar 30, 2021

azawlocki deleted the integration-tests branch March 30, 2021 10:28

azawlocki mentioned this pull request Mar 31, 2021

Integration tests using goth #307

Merged

azawlocki linked an issue Apr 8, 2021 that may be closed by this pull request

CI for yapapi using goth #280

Closed

azawlocki mentioned this pull request Apr 8, 2021

CI for yapapi using goth #280

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration test that starts blender example in goth #281

Add integration test that starts blender example in goth #281

azawlocki commented Mar 5, 2021 •

edited

Loading

filipgolem Mar 10, 2021 •

edited

Loading

azawlocki Mar 29, 2021

kmazurek left a comment

kmazurek Mar 26, 2021

azawlocki Mar 29, 2021

kmazurek Mar 26, 2021

kmazurek Mar 26, 2021

azawlocki Mar 29, 2021

filipgolem Mar 29, 2021

azawlocki Mar 29, 2021

filipgolem Mar 29, 2021

azawlocki Mar 29, 2021

shadeofblue Mar 29, 2021 •

edited

Loading

kmazurek Mar 29, 2021

shadeofblue Mar 29, 2021

azawlocki Mar 29, 2021 •

edited

Loading

shadeofblue Mar 29, 2021

shadeofblue left a comment

	# The tokens `~` and `~user` are also replaced by the corresponding users's
	# The tokens `~` and `~user` are also replaced by user's

	image: docker.pkg.github.com/golemfactory/yagna-zksync/yagna-zksync-mock:7a25ed913d4a
	image: docker.pkg.github.com/golemfactory/yagna-zksync/yagna-zksync-mock:f6d0cf02f6bc

	class: "goth.runner.provider.ProviderProbeWithLogSteps"
	class: "goth.runner.provider.ProviderProbe"

Add integration test that starts blender example in goth #281

Add integration test that starts blender example in goth #281

Conversation

azawlocki commented Mar 5, 2021 • edited Loading

filipgolem Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmazurek 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

Choose a reason for hiding this comment

shadeofblue Mar 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azawlocki Mar 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shadeofblue left a comment

Choose a reason for hiding this comment

azawlocki commented Mar 5, 2021 •

edited

Loading

filipgolem Mar 10, 2021 •

edited

Loading

shadeofblue Mar 29, 2021 •

edited

Loading

azawlocki Mar 29, 2021 •

edited

Loading