add build file for github actions #1622

m00sey · 2020-10-03T16:07:15Z

This PR is superseded by #1645

~~Jobs for indy-common/node, lint and @pytest.mark check.~~

~~In order to parallelize the build, I created a build matrix for each of the "test" packages.~~
~~Each test is annotated with @pytest.mark<package>~~

~~The pytest_checks job returns an array of all found unique decorators and uses that as the build matrix.~~

~~The build will fail if any test_ method is missing a mark decorator.~~

~~This takes the build time from >45 minutes to 12 minutes.~~

Signed-off-by: Kevin Griffin griffin.kev@gmail.com

sovbot · 2020-10-03T16:07:17Z

Can one of the admins verify this patch?

Toktar · 2020-11-10T13:15:53Z

You have done an amazing job! Thank you.
But just in case, I want to warn that Plenum https://github.com/hyperledger/indy-plenum is a large part of Indy Node, they are inextricably linked. And it might be more convenient to start with it.

lgtm-com · 2020-12-19T01:17:01Z

This pull request introduces 1 alert and fixes 4 when merging 52c4203 into b9c21c8 - view on LGTM.com

new alerts:

1 for Syntax error

fixed alerts:

2 for Signature mismatch in overriding method
1 for Clear-text logging of sensitive information
1 for __init__ method calls overridden method

WadeBarnes · 2021-01-09T14:36:04Z

setup.py

@@ -28,7 +28,7 @@
 BASE_DIR = os.path.join(os.path.expanduser("~"), ".indy")

 tests_require = ['attrs==19.1.0', 'pytest==3.3.1', 'pytest-xdist==1.22.1', 'pytest-forked==0.2',
-                 'python3-indy==1.13.0-dev-1420', 'pytest-asyncio==0.8.0']
+                 'python3-indy==1.15.0', 'pytest-asyncio==0.8.0']


@jovfer, @askolesov, @Toktar, What is your opinion on this? Do you anticipate any unforeseen issues here.

I think it's ok if tests are passing

@WadeBarnes, @m00sey I really like this idea. But if we change the dependency here, I suggest changing them in ci/uduntu.dockerfile to preservation of consistency even if this file will be deprecated soon.
And it's a bit strange if indy-node and indy-plenum will depend on different libindy packages. I guess it's absolutely unimportant because of it's used just for a test environment, but still.

WadeBarnes

@m00sey, Looking good! Just a few references to update that I can see.

I'd also like to see some documentation indicating which Jenkins pipeline flow(s) this replaces, to make it easier to:

Identify the Jenkins workflows being replaced.
Review and compare the Jenkins vs GHA flows to verify everything is covered in the new workflows.
Decommission the Jenkins workflows when we're satisfied we have equivalent or better GHA flows.

.github/workflows/build.yaml

WadeBarnes · 2021-01-09T14:54:06Z

(ci) test this please

WadeBarnes · 2021-01-11T14:32:06Z

@m00sey, I think some documentation around how to build and manage the indy-node-build container used by the actions would be useful too.

lgtm-com · 2021-01-13T17:15:00Z

This pull request introduces 1 alert when merging 5b9a03f into 5a68647 - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2021-01-13T17:59:31Z

This pull request introduces 1 alert when merging fef433c into 5a68647 - view on LGTM.com

new alerts:

1 for Unused import

…common/node and lint. Signed-off-by: Kevin Griffin <griffin.kev@gmail.com>

Toktar · 2021-02-03T15:45:31Z

indy_node/test/auth_rule/auth_framework/test_auth_rule_using.py

+    @pytest.mark.auth_framework
+    def test_auth_rule_add_new_trustee(self, env):
+        self.runner(AddNewTrusteeTest(env, auth_map.add_new_trustee.get_action_id()))
+


Tests for the auth map have already been divided into different ones, since test classes were supplied as parameters to the auth_rule_tests. Thus, I see no point in making such changes. Personally, it seems to me that now it looks cumbersome, but it's a matter of taste. Another interesting point is that despite the possible improvement in terms of parallelization of tests, their order will be changed. Unfortunately, this can be critical to test passing and lead to unexpected crashes that are nearly impossible to replicate for research. I agree It's a very upset that our tests have this problem, but it's a fact.
Thus, such a change looks ok to me, but in theory it can do some harm.

So the problem with this test was that the single test, output over 30,000 lines of logs, and is being pushed to the surefire plugin as a single string.

This caused the surefire plugin to break.

The suite has been run many times, without an ordering failure.

However I understand your concern, if we could keep the separate test invocations, but, use pytest.mark to keep order, would that satisfy your concern?

Got it. Sure, with a same order it should permanently work.

Toktar · 2021-02-03T16:17:10Z

setup.py

@@ -28,7 +28,7 @@
 BASE_DIR = os.path.join(os.path.expanduser("~"), ".indy")

 tests_require = ['attrs==19.1.0', 'pytest==3.3.1', 'pytest-xdist==1.22.1', 'pytest-forked==0.2',
-                 'python3-indy==1.13.0-dev-1420', 'pytest-asyncio==0.8.0']
+                 'python3-indy==1.15.0', 'pytest-asyncio==0.8.0']


@WadeBarnes, @m00sey I really like this idea. But if we change the dependency here, I suggest changing them in ci/uduntu.dockerfile to preservation of consistency even if this file will be deprecated soon.
And it's a bit strange if indy-node and indy-plenum will depend on different libindy packages. I guess it's absolutely unimportant because of it's used just for a test environment, but still.

Toktar · 2021-02-03T16:26:07Z

.github/workflows/build.yaml

+        continue-on-error: true
+
+      - name: Run Indy Node ${{ matrix.module }} tests
+        run: python3 -m pytest -l -m ${{ matrix.module }} -vv --junitxml=test-result-node-${{ matrix.module }}.xml indy_node


The implementation of parallelizing tests looks pretty cool.
We already have runner.py for parallelizing tests. It would be great to replace it with something better, but maybe we could do it in the different PRs with adding a GitHub actions to make changes gradually?

I'm not sure how runner.py would parallelize in the same way, given this issues a run command for each permutation in the matrix, runner.py would spawn a single runner.

It doesn't parallelize in the same way, but runner.py has --test-only-slice to run only some part of tests dividing them on directories.
Example of using runner.py from indy-plenum CI:
RUSTPYTHONASYNCIODEBUG=0 python runner.py --pytest python -m pytest -l -vv --dir plenum --output test-result-plenum-1.jenkinsubuntu01.txt --test-only-slice 1/3
where 1/3 is a part which we want to launch.

That particular runner and usage is already a part of the plenum GHA PR.

I will look into the usage of slice for node.

Toktar · 2021-02-04T23:33:45Z

[for CI] Test this please

WadeBarnes · 2021-02-07T20:00:39Z

Closed in favor of #1645

m00sey requested review from lampkin-diet, ashcherbakov, skhoroshavin and Toktar as code owners October 3, 2020 16:07

m00sey marked this pull request as draft October 3, 2020 16:07

WadeBarnes self-requested a review October 3, 2020 16:37

m00sey force-pushed the ci-update branch from 7ad26f5 to cb45683 Compare October 4, 2020 18:05

m00sey force-pushed the ci-update branch from 0a1fd09 to 9b061dd Compare November 6, 2020 02:20

m00sey marked this pull request as ready for review November 6, 2020 02:27

m00sey force-pushed the ci-update branch from 9b061dd to aeeba87 Compare November 6, 2020 02:33

m00sey changed the title ~~Added config for github actions, introducing three jobs.~~ add build file for github actions Nov 6, 2020

m00sey force-pushed the ci-update branch 5 times, most recently from 7218391 to cb09e02 Compare November 12, 2020 03:19

m00sey mentioned this pull request Nov 12, 2020

adds support for github actions hyperledger/indy-plenum#1497

Closed

m00sey force-pushed the ci-update branch 3 times, most recently from 1fdd18b to 52c4203 Compare December 19, 2020 01:08

m00sey force-pushed the ci-update branch 3 times, most recently from 5e68235 to bf0d644 Compare January 8, 2021 16:18

WadeBarnes reviewed Jan 9, 2021

View reviewed changes

WadeBarnes requested changes Jan 9, 2021

View reviewed changes

.github/workflows/build.yaml Outdated Show resolved Hide resolved

m00sey force-pushed the ci-update branch 4 times, most recently from a06205e to 5b9a03f Compare January 13, 2021 16:56

m00sey force-pushed the ci-update branch from 5b9a03f to fef433c Compare January 13, 2021 17:41

m00sey force-pushed the ci-update branch from fef433c to 12b9217 Compare January 14, 2021 15:02

m00sey requested a review from WadeBarnes January 18, 2021 21:43

m00sey force-pushed the ci-update branch from 12b9217 to 1389a80 Compare January 22, 2021 01:17

m00sey force-pushed the ci-update branch from 1389a80 to 73f8ddf Compare February 1, 2021 18:23

Adds config for gihub actions, introducing three jobs. Jobs for indy-…

587008c

…common/node and lint. Signed-off-by: Kevin Griffin <griffin.kev@gmail.com>

m00sey force-pushed the ci-update branch from 73f8ddf to 587008c Compare February 1, 2021 18:49

Toktar reviewed Feb 3, 2021

View reviewed changes

m00sey marked this pull request as draft February 5, 2021 21:40

m00sey mentioned this pull request Feb 6, 2021

adds GitHub Actions and build containers #1645

Merged

WadeBarnes closed this Feb 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add build file for github actions #1622

add build file for github actions #1622

m00sey commented Oct 3, 2020 •

edited

Loading

sovbot commented Oct 3, 2020

Toktar commented Nov 10, 2020

lgtm-com bot commented Dec 19, 2020

WadeBarnes Jan 9, 2021

askolesov Jan 10, 2021

WadeBarnes Jan 11, 2021

Toktar Feb 3, 2021

WadeBarnes left a comment

WadeBarnes commented Jan 9, 2021

WadeBarnes commented Jan 11, 2021

lgtm-com bot commented Jan 13, 2021

lgtm-com bot commented Jan 13, 2021

Toktar Feb 3, 2021

m00sey Feb 4, 2021

Toktar Feb 4, 2021

Toktar Feb 3, 2021

Toktar Feb 3, 2021

m00sey Feb 4, 2021

Toktar Feb 4, 2021

m00sey Feb 4, 2021

m00sey Feb 4, 2021

Toktar commented Feb 4, 2021

WadeBarnes commented Feb 7, 2021

add build file for github actions #1622

add build file for github actions #1622

Conversation

m00sey commented Oct 3, 2020 • edited Loading

sovbot commented Oct 3, 2020

Toktar commented Nov 10, 2020

lgtm-com bot commented Dec 19, 2020

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

WadeBarnes left a comment

Choose a reason for hiding this comment

WadeBarnes commented Jan 9, 2021

WadeBarnes commented Jan 11, 2021

lgtm-com bot commented Jan 13, 2021

lgtm-com bot commented Jan 13, 2021

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

Toktar commented Feb 4, 2021

WadeBarnes commented Feb 7, 2021

m00sey commented Oct 3, 2020 •

edited

Loading