Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/adding_instrumentail_tests #1561

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

stepanLav
Copy link
Member

@stepanLav stepanLav commented Jun 27, 2024

[Still in process]
That PR adds instrumental tests to the PR's workflow

@stepanLav stepanLav force-pushed the feat/adding_instrumentail_tests branch from d911388 to 8d9e55a Compare June 27, 2024 06:49
fix order

add unique build

remove unit tests

fix run tests

fix build tests

remove example test which lead to error

temp fix for build

remove example instrumental tests

adding debug lines

fix debug

fix path

fix build type

fix path

build debug version

fix path

run on github action

fix balances tests

store results in separate repo

fix artifacts path

fix artifact version

fix download path

fix path

fix path

update secrets

fix access

remove unnecessary checkout

update secrets

update storing folder

update deployment dir
@stepanLav stepanLav force-pushed the feat/move_tests_to_main_repo branch from 216aa97 to 07b8301 Compare June 27, 2024 06:53
@stepanLav stepanLav force-pushed the feat/adding_instrumentail_tests branch from 8d9e55a to 3d3f062 Compare June 27, 2024 06:58
fix TestSigner
@stepanLav stepanLav force-pushed the feat/move_tests_to_main_repo branch from 07b8301 to 03f2a0c Compare June 27, 2024 07:00
@stepanLav stepanLav force-pushed the feat/adding_instrumentail_tests branch from 3d3f062 to 30b1f5f Compare June 27, 2024 07:00
@stepanLav stepanLav marked this pull request as draft June 27, 2024 07:25
Base automatically changed from feat/move_tests_to_main_repo to develop July 5, 2024 06:17
Comment on lines +29 to +33
def run():
os.system('adb wait-for-device')
p = sp.Popen('adb shell am instrument -w -m -e notClass io.novafoundation.nova.balances.BalancesIntegrationTest -e package io.novafoundation.nova.debug io.novafoundation.nova.debug.test/io.qameta.allure.android.runners.AllureAndroidJUnitRunner',
shell=True, stdout=sp.PIPE, stderr=sp.PIPE, stdin=sp.PIPE)
return p.communicate()
Copy link
Contributor

Choose a reason for hiding this comment

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

As I see we start only BalancesIntegrationTest. Is it correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why we need to run balances test separately?
Is it because this test is moved to another directory?

@@ -50,13 +55,37 @@ class MoonbaseSendIntagrationTest {
return EthereumKeypairFactory.generate(seed, junctions)
}

private inner class TestSigner(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice solution I guess! But let's move inner class to the bottom of its parent

@@ -50,13 +55,37 @@ class MoonbaseSendIntagrationTest {
return EthereumKeypairFactory.generate(seed, junctions)
}

private inner class TestSigner(
private val testPayload: (SignerPayloadExtrinsic) -> Unit
Copy link
Contributor

Choose a reason for hiding this comment

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

As I see we don't use testPayload from outside, so we can remove this redundant variable

private val testPayload: (SignerPayloadExtrinsic) -> Unit
) : NovaSigner {

val accountId = ByteArray(32) { 1 }
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems we don't use it but to be semantically correct let's provide accountId to the constructor from outside

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants