Skip to content

Tests/e2e tests staging#1943

Merged
open-junius merged 53 commits intostagingfrom
tests/e2e-tests-staging
Jun 13, 2024
Merged

Tests/e2e tests staging#1943
open-junius merged 53 commits intostagingfrom
tests/e2e-tests-staging

Conversation

@open-junius
Copy link
Contributor

@open-junius open-junius commented May 27, 2024

The PR will fix #1932.
Some explanation for the implementation.

  1. currently, the output of command includes more lines than expected because of bittensor version. We can see the message like Please update to the latest version at your earliest convenience. Run the following command to upgrade. that why assert len(lines) >= used to check the output.
  2. The root network weight set is disable, so some commands are commented in the test case. Will refactor after new implementation is available in bittensor side.
  3. The test for delegate stake is commented because we can't get the delegate data via subtensor.get_delegates method.

@open-junius open-junius self-assigned this May 27, 2024
@open-junius open-junius marked this pull request as ready for review June 11, 2024 04:22
import bittensor


# we can't test it now since the root network weights can't be set
Copy link

@opendansor opendansor Jun 11, 2024

Choose a reason for hiding this comment

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

If we cant test this right now, should we remove it until we can properly test it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pls keep it in the repo. I need some time to try, then know what's the correct parameter and how to check.

Choose a reason for hiding this comment

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

Can you keep this in a separate branch? There is no need to push this in while its still under development and not ready to go out to the public. Every test we run on the Bittensor side actually tests what it is supposed to test. This will add tests that don't actually test what they claim to test, and we will be required to "remember" that X or Y test is incomplete, giving us holes in what bugs and issues can go unnoticed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will remove it.

assert delegates == 11796

# stake 1 TAO
# exec_command(

Choose a reason for hiding this comment

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

What is the purpose of these commented out blocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to add and reduce stake

Choose a reason for hiding this comment

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

I understand, but the code is commented out... Meaning the test doesn't actually add or reduce stake, and test that stake is added or reduced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, will just keep it in my local branch

from ...utils import new_wallet, sudo_call_set_network_limit


# we can't test it now since the root network weights can't be set

Choose a reason for hiding this comment

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

Does this test run correctly? Maybe we write this test when the command is updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The extrinsic in subtensor side is updated. From @distributedstatemachine , I know cortex team will change accordingly.

assert len(lines) >= 4
bittensor.logging.info(lines)

# assert "Root Network" in lines[0]

Choose a reason for hiding this comment

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

Why are these lines commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I mentioned in the description, we always get the extra output about bittensor version. so just commented out, can add them back after version is aligned.

Copy link

@opendansor opendansor left a comment

Choose a reason for hiding this comment

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

Please add tests that actually test what they claim to test. If X or Y test is still under development, keep it in a git branch and work on it as you see fit, but there is no need to push in tests that don't accomplish what they set out to do.

@open-junius open-junius requested a review from opendansor June 12, 2024 14:07
@open-junius open-junius merged commit d139338 into staging Jun 13, 2024
@open-junius open-junius deleted the tests/e2e-tests-staging branch June 13, 2024 02:44
@ibraheem-abe ibraheem-abe mentioned this pull request Aug 23, 2024
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.

5 participants