fix: disable life support for nearlib test #7667
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
What this test is intended to do is to spawn a local neard node and to run near-api-js tests against that. The purpose is to detect unintentional changes to near public API. We have disabled actually running the test a while ago, and today the build broke b/c api-js moved from yarn to pnpm. I think at this it's safe to kill this infra completely, as we weren't running it for a while. It would of course be nice to have some in-repo tests covering public JSON API, but I think we are currently server well-enough by betanet and testnet anyway. Testing public APIs via running tests of downstream projects upstream is clearly a wrong way to do that!
matklad
force-pushed
the
m/upbreak-tests
branch
from
988f66b
to
e04cf94
Compare
|
@morgsmccauley (random person from near-api-js), how is near-api-js is tested? Specifically, when running the tests in CI, do you also run the node, or is it fully mocked? |
nikurt
approved these changes
Sep 22, 2022
|
@matklad it uses |
|
Thanks! So this means that, if we do break our APIs, we'll notice this at least when we hit testnet. This makes me confident that we are somewhat covered here. |
nikurt
pushed a commit
that referenced
this pull request
Sep 26, 2022
What this test is intended to do is to spawn a local neard node and to run near-api-js tests against that. The purpose is to detect unintentional changes to near public API. We have disabled actually running the test a while ago, and today the build broke b/c api-js moved from yarn to pnpm. I think at this it's safe to kill this infra completely, as we weren't running it for a while. It would of course be nice to have some in-repo tests covering public JSON API, but I think we are currently server well-enough by betanet and testnet anyway. Testing public APIs via running tests of downstream projects upstream is clearly a wrong way to do that!
matklad
commented
Sep 29, 2022
| # subprocess.check_output(['docker', 'pull', 'v2tec/watchtower']) | ||
| # except subprocess.CalledProcessError as exc: | ||
| # print("Failed to fetch docker containers: %s" % exc) | ||
| # exit(1) |
There was a problem hiding this comment.
Ouch, this definitely wasn't meant to be commited, sorry.
matklad
added a commit
to matklad/nearcore
that referenced
this pull request
Sep 29, 2022
I didn't mean to comment the code in setup_and_run, and I did mean to remove wait_on_server, as that was only used by the test removed in near#7667
Merged
nikurt
pushed a commit
that referenced
this pull request
Nov 9, 2022
What this test is intended to do is to spawn a local neard node and to run near-api-js tests against that. The purpose is to detect unintentional changes to near public API. We have disabled actually running the test a while ago, and today the build broke b/c api-js moved from yarn to pnpm. I think at this it's safe to kill this infra completely, as we weren't running it for a while. It would of course be nice to have some in-repo tests covering public JSON API, but I think we are currently server well-enough by betanet and testnet anyway. Testing public APIs via running tests of downstream projects upstream is clearly a wrong way to do that!
nikurt
pushed a commit
that referenced
this pull request
Nov 9, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this test is intended to do is to spawn a local neard node and to run near-api-js tests against that.
The purpose is to detect unintentional changes to near public API.
We have disabled actually running the test a while ago, and today the build broke b/c api-js moved from yarn to pnpm.
I think at this it's safe to kill this infra completely, as we weren't running it for a while.
It would of course be nice to have some in-repo tests covering public JSON API, but I think we are currently server well-enough by betanet and testnet anyway.
Testing public APIs via running tests of downstream projects upstream is clearly a wrong way to do that!