-
Notifications
You must be signed in to change notification settings - Fork 59
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
Test the https-availability nibble-query #4028
Test the https-availability nibble-query #4028
Conversation
Use or-join to make the tests pass
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.
Nice work. I'm no expert in the queries, but did take a look at the tests.
While looking at this I had a couple of thoughts for potential improvements or something to think about (for later):
ip_generator
could be a nice fixture on 'function' scope level to be used by multiple teststest_https_availability_query
might be a good example for test parameterization, but in this case maybe too complex/ abstract
|
||
if os.environ.get("CI") != "1": | ||
pytest.skip("Needs XTDB multinode container.", allow_module_level=True) | ||
|
||
STATIC_IP = ".".join((4 * "1 ").split()) | ||
STATIC_IP = ".".join(4 * ["1"]) |
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.
Could we just write 1.1.1.1 here?
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 reason we write it that way is not to have SonarCloud report a "static ip security vulnerability".
…es-https-availability-query
|
||
for _ in range(12): | ||
create_port(xtdb_ooi_repository, next(ip), 443, valid_time) | ||
assert xtdb_ooi_repository.session.client.query(query)[0][-1] == 13 |
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.
@originalsouth Or should this be 1 since the ips we created are not attached to the hostname through the website?
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.
Perhaps the ipaddress should be a parameter in the or-join, I'll check.
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 think the query breaks with more than one targets, let me check...
…ity-query' into fix/test-nibbles-https-availability-query # Conflicts: # octopoes/tests/integration/test_https_availability_nibble.py
…es-https-availability-query
Quality Gate failedFailed conditions |
5c8cd8a
into
feature/nibbles-https-availability
Changes
Small fix merging in a feature branch.