-
Notifications
You must be signed in to change notification settings - Fork 593
Smoketests can run against remote servers #3012
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
base: master
Are you sure you want to change the base?
Conversation
bottest please |
…er' into bfops/release-smoketests
@@ -1,4 +1,4 @@ | |||
default_server = "127.0.0.1:3000" | |||
default_server = "localhost" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of using a hardcoded address, use a nickname for the server specified below. This allows us to look up the correct protocol
for that server.
@@ -229,6 +229,7 @@ def tearDown(self): | |||
self.docker.compose("up", "-d") | |||
super().tearDown() | |||
|
|||
# TODO: This function seems to run even when `--docker` is not passed, leading to errors unless `-x replication` is passed, due to the docker-related code below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty unrelated, but I ran into this while testing
@@ -10,8 +10,6 @@ def setUp(self): | |||
def test_call(self): | |||
"""Ensure that anyone has the permission to call any standard reducer""" | |||
|
|||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this new_identity
really added anything to the test, but it hamstrung the test when running against servers that required a spacetime login
.
@@ -21,28 +19,23 @@ def test_call(self): | |||
def test_delete(self): | |||
"""Ensure that you cannot delete a database that you do not own""" | |||
|
|||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this new_identity
really added anything to the test, but it hamstrung the test when running against servers that required a spacetime login
.
with self.assertRaises(Exception): | ||
self.spacetime("delete", self.database_identity) | ||
|
||
def test_describe(self): | ||
"""Ensure that anyone can describe any database""" | ||
|
||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this new_identity
really added anything to the test, but it hamstrung the test when running against servers that required a spacetime login
.
self.publish_module() | ||
|
||
self.reset_config() | ||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the goal of the reset_config
was to be using a different identity than the publish
. This accomplishes that directly.
self.publish_module() | ||
|
||
self.reset_config() | ||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed these in favor of just using --anonymous
|
||
def test_logs(self): | ||
"""Ensure that we are not able to view the logs of a module that we don't have permission to view""" | ||
|
||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this new_identity
really added anything to the test (because we just use new_identity
to change identities below anyway), but it hamstrung the test when running against servers that required a spacetime login
to publish.
@@ -57,10 +49,9 @@ def test_logs(self): | |||
def test_publish(self): | |||
"""This test checks to make sure that you cannot publish to an identity that you do not own.""" | |||
|
|||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this new_identity
really added anything to the test, but it hamstrung the test when running against servers that required a spacetime login
.
self.publish_module() | ||
|
||
self.reset_config() | ||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the goal of this reset_config
was effectively to switch identities, which we do directly now
@@ -73,11 +64,10 @@ def test_publish(self): | |||
def test_replace_names(self): | |||
"""Test that you can't replace names of a database you don't own""" | |||
|
|||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this new_identity
really added anything to the test, but it hamstrung the test when running against servers that required a spacetime login
.
name = random_string() | ||
self.publish_module(name) | ||
|
||
self.reset_config() | ||
self.new_identity() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the goal of this reset_config
was effectively to switch identities, which we do directly now
token = config['spacetimedb_token'] | ||
conn = http.client.HTTPConnection(host) | ||
server_name = config['default_server'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this corresponds to the change to the config.toml
below; now the default_server
is expected to be a server nickname instead of a hostname. This allows us to 1) avoid duplicating the hostname whenever we want to update it, and 2) look up the protocol associated with this hostname (which we previously didn't have a way of providing).
Description of Changes
This enables smoketests to run against remote servers, such as maincloud / maincloud staging.
I also added a
--spacetime-login
param, for servers that require a "proper" spacetime login (such as both servers above).Usage:
python3 -m smoketests \ --remote-server https://maincloud.staging.spacetimedb.com \ --spacetime-login \ -x replication # for some reason this is required, even though I swear it should be disabled by not passing `--docker`
API and ABI breaking changes
None. CI only.
Expected complexity level and risk
1
Testing