Check HTTP version and status first, then other asserts

CHANGELOG.md

@@ -8,12 +8,15 @@ Thanks to
 [@jmoore34](https://github.com/jmoore34),
 [@devnoname120](https://github.com/devnoname120),
 [@dalejefferson-rnf](https://github.com/dalejefferson-rnf),
+[@dnsmichi](https://github.com/dnsmichi),
 
 
 Enhancements:
 
 * Add name attribute to JUnit report [#1078](https://github.com/Orange-OpenSource/hurl/issues/1078)
 
+* Check HTTP version and status first, then other asserts. [#1072](https://github.com/Orange-OpenSource/hurl/issues/1072)
+
 * Support new one line string [#1041](https://github.com/Orange-OpenSource/hurl/issues/1041)
 
 * Add filters for htmlEscape and htmlUnescape [#1038](https://github.com/Orange-OpenSource/hurl/issues/1038)

integration/tests_failed/assert_status.err

@@ -1,7 +1,7 @@
 error: Assert status code
-  --> tests_failed/assert_status.hurl:2:6
+  --> tests_failed/assert_status.hurl:9:6
    |
- 2 | HTTP 200
+ 9 | HTTP 200
    |      ^^^ actual value is <404>
    |
 

integration/tests_failed/assert_status.html

@@ -1,4 +1,16 @@
-<pre><code class="language-hurl"><span class="hurl-entry"><span class="request"><span class="line"><span class="method">GET</span> <span class="url">http://localhost:8000/not_found</span></span>
-</span><span class="response"><span class="line"><span class="version">HTTP</span> <span class="number">200</span></span>
-</span></span><span class="line"></span>
-</code></pre>
+<pre><code class="language-hurl"><span class="hurl-entry"><span class="request"><span class="line"></span><span class="comment"># We add various explicit asserts on the body, and</span>
+<span class="line"></span><span class="comment"># implicit asserts on the response headers.</span>
+<span class="line"></span><span class="comment"># As the status code is not correct, those asserts</span>
+<span class="line"></span><span class="comment"># should not be tested (the status code is a "stronger"</span>
+<span class="line"></span><span class="comment"># assert than the others).</span>
+<span class="line"></span>
+<span class="line"><span class="method">GET</span> <span class="url">http://localhost:8000/not_found</span></span>
+</span><span class="response"><span class="line"></span>
+<span class="line"><span class="version">HTTP</span> <span class="number">200</span></span>
+<span class="line"><span class="string">x-baz</span><span>:</span> <span class="string">xxx</span></span>
+<span class="line section-header">[Asserts]</span>
+<span class="line"><span class="query-type">duration</span> <span class="predicate-type">&lt;</span> <span class="number">0</span></span>
+<span class="line"><span class="query-type">jsonpath</span> <span class="string">"$.foo"</span> <span class="predicate-type">startsWith</span> <span class="string">"something"</span></span>
+<span class="line"><span class="query-type">jsonpath</span> <span class="string">"$.bar"</span> <span class="not">not</span> <span class="predicate-type">exists</span></span>
+<span class="line"><span class="query-type">header</span> <span class="string">"x-bar"</span> <span class="predicate-type">exists</span></span>
+</span></span></code></pre>

integration/tests_failed/assert_status.hurl

@@ -1,3 +1,15 @@
+# We add various explicit asserts on the body, and
+# implicit asserts on the response headers.
+# As the status code is not correct, those asserts
+# should not be tested (the status code is a "stronger"
+# assert than the others).
+
 GET http://localhost:8000/not_found
-HTTP 200
 
+HTTP 200
+x-baz: xxx
+[Asserts]
+duration < 0
+jsonpath "$.foo" startsWith "something"
+jsonpath "$.bar" not exists
+header "x-bar" exists

integration/tests_failed/assert_status.json

@@ -1 +1 @@
-{"entries":[{"request":{"method":"GET","url":"http://localhost:8000/not_found"},"response":{"status":200}}]}
+{"entries":[{"request":{"method":"GET","url":"http://localhost:8000/not_found"},"response":{"status":200,"headers":[{"name":"x-baz","value":"xxx"}],"asserts":[{"query":{"type":"duration"},"predicate":{"type":"less","value":0}},{"query":{"type":"jsonpath","expr":"$.foo"},"predicate":{"type":"start-with","value":"something"}},{"query":{"type":"jsonpath","expr":"$.bar"},"predicate":{"not":true,"type":"exist"}},{"query":{"type":"header","name":"x-bar"},"predicate":{"type":"exist"}}]}}]}

integration/tests_failed/assert_status.out.pattern

@@ -1 +1 @@
-{"cookies":[],"entries":[{"asserts":[{"line":2,"success":true},{"line":2,"message":"Assert status code\n  --> tests_failed/assert_status.hurl:2:6\n   |\n 2 | HTTP 200\n   |      ^^^ actual value is <404>\n   |","success":false}],"calls":[{"request":{"cookies":[],"headers":[{"name":"Host","value":"localhost:8000"},{"name":"Accept","value":"*/*"},{"name":"User-Agent","value":"hurl/~~~"}],"method":"GET","queryString":[],"url":"http://localhost:8000/not_found"},"response":{"cookies":[],"headers":[{"name":"Content-Type","value":"text/html; charset=utf-8"},{"name":"Content-Length","value":"232"},{"name":"Server","value":"Flask Server"},{"name":"Date","value":"~~~"}],"httpVersion":"HTTP/1.0","status":404}}],"captures":[],"index":1,"time":~~~}],"filename":"tests_failed/assert_status.hurl","success":false,"time":~~~}
+{"cookies":[],"entries":[{"asserts":[{"line":9,"success":true},{"line":9,"message":"Assert status code\n  --> tests_failed/assert_status.hurl:9:6\n   |\n 9 | HTTP 200\n   |      ^^^ actual value is <404>\n   |","success":false}],"calls":[{"request":{"cookies":[],"headers":[{"name":"Host","value":"localhost:8000"},{"name":"Accept","value":"*/*"},{"name":"User-Agent","value":"hurl/~~~"}],"method":"GET","queryString":[],"url":"http://localhost:8000/not_found"},"response":{"cookies":[],"headers":[{"name":"Content-Type","value":"text/html; charset=utf-8"},{"name":"Content-Length","value":"232"},{"name":"Server","value":"Flask Server"},{"name":"Date","value":"~~~"}],"httpVersion":"HTTP/1.0","status":404}}],"captures":[],"index":1,"time":~~~}],"filename":"tests_failed/assert_status.hurl","success":false,"time":~~~}

packages/hurl/src/runner/entry.rs

@@ -29,6 +29,7 @@ use super::request::eval_request;
 use super::response::{eval_asserts, eval_captures};
 use super::value::Value;
 use crate::runner::request::{cookie_storage_clear, cookie_storage_set};
+use crate::runner::response::eval_version_status_asserts;
 use crate::runner::runner_options::RunnerOptions;
 use crate::runner::template::eval_template;
 
@@ -124,7 +125,32 @@ pub fn run(
         .collect();
     let time_in_ms = calls.iter().map(|c| c.response.duration.as_millis()).sum();
 
-    // Compute captures
+    // We proceed asserts and captures in this order:
+    // 1. first, check implicit assert on status and version. If KO, test is failed
+    // 2. then, we compute captures, we might need them in asserts
+    // 3. finally, run the remaining asserts
+    let mut all_asserts = vec![];
+
+    if !runner_options.ignore_asserts {
+        if let Some(response_spec) = &entry.response {
+            let mut asserts = eval_version_status_asserts(response_spec, http_response);
+            let errors = asserts_to_errors(&asserts);
+            if !errors.is_empty() {
+                logger.debug("");
+                return EntryResult {
+                    entry_index,
+                    calls,
+                    captures: vec![],
+                    asserts,
+                    errors,
+                    time_in_ms,
+                    compressed: client_options.compressed,
+                };
+            }
+            all_asserts.append(&mut asserts);
+        }
+    };
+
     let captures = match &entry.response {
         None => vec![],
         Some(response_spec) => match eval_captures(response_spec, http_response, variables) {
@@ -134,7 +160,7 @@ pub fn run(
                     entry_index,
                     calls,
                     captures: vec![],
-                    asserts: vec![],
+                    asserts: all_asserts,
                     errors: vec![e],
                     time_in_ms,
                     compressed: client_options.compressed,
@@ -152,21 +178,34 @@ pub fn run(
     logger.debug("");
 
     // Compute asserts
-    let asserts = if runner_options.ignore_asserts {
-        vec![]
-    } else {
-        match &entry.response {
-            None => vec![],
-            Some(response_spec) => eval_asserts(
+    if !runner_options.ignore_asserts {
+        if let Some(response_spec) = &entry.response {
+            let mut asserts = eval_asserts(
                 response_spec,
                 variables,
                 http_response,
                 &runner_options.context_dir,
-            ),
+            );
+            all_asserts.append(&mut asserts);
         }
     };
 
-    let errors = asserts
+    let errors = asserts_to_errors(&all_asserts);
+
+    EntryResult {
+        entry_index,
+        calls,
+        captures,
+        asserts: all_asserts,
+        errors,
+        time_in_ms,
+        compressed: client_options.compressed,
+    }
+}
+
+/// Converts a list of [`AssertResult`] to a list of [`Error`].
+fn asserts_to_errors(asserts: &[AssertResult]) -> Vec<Error> {
+    asserts
         .iter()
         .filter_map(|assert| assert.error())
         .map(
@@ -178,17 +217,7 @@ pub fn run(
                 assert: true,
             },
         )
-        .collect();
-
-    EntryResult {
-        entry_index,
-        calls,
-        captures,
-        asserts,
-        errors,
-        time_in_ms,
-        compressed: client_options.compressed,
-    }
+        .collect()
 }
 
 impl From<&RunnerOptions> for ClientOptions {

packages/hurl/src/runner/response.rs

@@ -30,12 +30,11 @@ use super::json::eval_json_value;
 use super::template::eval_template;
 use super::value::Value;
 
-/// Returns a list of assert results, given a set of `variables`, an actual `http_response` and a spec `response`.
-pub fn eval_asserts(
+/// Returns a list of assert results on the response status code and HTTP version,
+/// given a set of `variables`, an actual `http_response` and a spec `response`.
+pub fn eval_version_status_asserts(
     response: &Response,
-    variables: &HashMap<String, Value>,
     http_response: &http::Response,
-    context_dir: &ContextDir,
 ) -> Vec<AssertResult> {
     let mut asserts = vec![];
 
@@ -54,6 +53,20 @@ pub fn eval_asserts(
             source_info: status.source_info.clone(),
         });
     }
+    asserts
+}
+
+/// Returns a list of assert results, given a set of `variables`, an actual `http_response` and a spec `response`.
+///
+/// Asserts on status and version and not run in this function, there are run with `eval_version_status_asserts`
+/// as they're semantically stronger.
+pub fn eval_asserts(
+    response: &Response,
+    variables: &HashMap<String, Value>,
+    http_response: &http::Response,
+    context_dir: &ContextDir,
+) -> Vec<AssertResult> {
+    let mut asserts = vec![];
 
     for header in response.headers.iter() {
         match eval_template(&header.value, variables) {
@@ -377,6 +390,26 @@ mod tests {
                 &http::xml_two_users_http_response(),
                 &context_dir,
             ),
+            vec![AssertResult::Explicit {
+                actual: Ok(Some(Value::Nodeset(2))),
+                source_info: SourceInfo::new(1, 14, 1, 27),
+                predicate_result: Some(Err(Error {
+                    source_info: SourceInfo::new(1, 0, 1, 0),
+                    inner: RunnerError::AssertFailure {
+                        actual: "2".to_string(),
+                        expected: "3".to_string(),
+                        type_mismatch: false,
+                    },
+                    assert: true,
+                })),
+            },]
+        );
+    }
+
+    #[test]
+    pub fn test_eval_version_status_asserts() {
+        assert_eq!(
+            eval_version_status_asserts(&user_response(), &http::xml_two_users_http_response(),),
             vec![
                 AssertResult::Version {
                     actual: String::from("HTTP/1.0"),
@@ -388,19 +421,6 @@ mod tests {
                     expected: 200,
                     source_info: SourceInfo::new(2, 10, 2, 13),
                 },
-                AssertResult::Explicit {
-                    actual: Ok(Some(Value::Nodeset(2))),
-                    source_info: SourceInfo::new(1, 14, 1, 27),
-                    predicate_result: Some(Err(Error {
-                        source_info: SourceInfo::new(1, 0, 1, 0),
-                        inner: RunnerError::AssertFailure {
-                            actual: "2".to_string(),
-                            expected: "3".to_string(),
-                            type_mismatch: false,
-                        },
-                        assert: true,
-                    })),
-                },
             ]
         );
     }