QA/webconnectivity.py: add test case for nonexistent domain #842
Conversation
The behavior of probe-engine is better than the one of MK. We have
disabled this check for MK and we should open an issue.
The following diff shows the difference in the toplevel keys:
```diff
--- MK.jsonl 2020-08-13 13:12:05.741791884 +0200
+++ OPE.jsonl 2020-08-13 13:12:14.092840549 +0200
@@ -1,9 +1,9 @@
{
"input": "http://antani.xyz",
"test_keys": {
- "accessible": true,
+ "accessible": null,
"agent": "redirect",
- "blocking": false,
+ "blocking": null,
"body_length_match": null,
"body_proportion": 0,
"client_resolver": "91.80.36.88",
@@ -16,15 +16,16 @@
"body_length": -1,
"failure": "dns_lookup_error",
"headers": {},
- "status_code": -1
+ "status_code": -1,
+ "title": ""
},
"tcp_connect": {}
},
"control_failure": null,
"dns_consistency": "consistent",
- "dns_experiment_failure": null,
+ "dns_experiment_failure": "dns_nxdomain_error",
"headers_match": null,
- "http_experiment_failure": "dns_lookup_error",
+ "http_experiment_failure": null,
"retries": null,
"socksproxy": null,
"status_code_match": null,
```
Because `antani.xyz` does not exist, OPE is clearly right here. I am also
wondering whether this issue with MK is related to the following issue:
https://github.com/ooni/probe-engine/issues/579
Maybe?
What I find rather interesting, also, is that there is no indication of
"dns_experiment_failure" in MK even though it clearly failed. Yet, we see
consistency in the "dns_consistency" key. But, still, I would argue it's
not so smart to elide the DNS failure in this case.
Also, note that the HTTP experiment is not even performed by OPE because
we really don't know what to do without IP addresses for the domain.
| assert isinstance(resp["code"], int) | ||
| if resp["headers"] is not None: | ||
| for key, value in resp["headers"].items(): | ||
| if tk["requests"]: |
There was a problem hiding this comment.
tk["requests"] is nil here with OPE
| for key, value in resp["headers"].items(): | ||
| assert isinstance(key, str) | ||
| common.check_maybe_binary_value(value) | ||
| if tk["tcp_connect"]: |
There was a problem hiding this comment.
tk["tcp_connect"] is nil here with OPE
|
So, after reading https://github.com/ooni/probe-engine/issues/579 more carefully, the test case we've introduced here is indeed the one originally reported by @andresazp. With OPE, this measurement is flagged as failed. Which is better than saying that all is good, because definitely it's not true that all is good here. I also think that the original suggestion by @andresazp is correct: we should treat this case specially. My plan is actually to first finish making sure we don't do worst than MK in many common measurement cases and then start addressing these corner cases by defining specific keys for identifying them. |
|
So, to recap, I'm merging this PR where the case in which the website is down is flagged as failed measurement. I think this is better than flagging as "all good" because users will be confused by that. The proper solution is to extend our mechanism of classification to also encompass this specific case, but that is something to do after the QA process is over. |
The behavior of probe-engine is better than the one of MK. We have
disabled this check for MK and we should open an issue.
The following diff shows the difference in the toplevel keys:
Because
antani.xyzdoes not exist, OPE is clearly right here. I am alsowondering whether this issue with MK is related to the following issue:
https://github.com/ooni/probe-engine/issues/579
Maybe?
What I find rather interesting, also, is that there is no indication of
"dns_experiment_failure" in MK even though it clearly failed. Yet, we see
consistency in the "dns_consistency" key. But, still, I would argue it's
not so smart to elide the DNS failure in this case.
Also, note that the HTTP experiment is not even performed by OPE because
we really don't know what to do without IP addresses for the domain.