[yaml-tests] Update test suite names, fix delete_indices, improve error reporting

Author: swallez

CONTRIBUTING.md

@@ -79,10 +79,10 @@ The `Elasticsearch` category of steps are specifically defined for this project
 
   - `STACK_VERSION`: Elasticsearch version like `7.9.0` or can be
   a snapshot release like `7.x-SNAPSHOT`
-  - `TEST_SUITE`: Elasticsearch distribution of `oss` or `xpack`
+  - `TEST_SUITE`: Elasticsearch distribution of `free` or `platinum`
 
   ```sh
-  cargo make test-yaml --env STACK_VERSION=7.9.0 --env TEST_SUITE=oss
+  cargo make test-yaml --env STACK_VERSION=7.9.0 --env TEST_SUITE=free
   ```
 
 ### Packages
@@ -198,4 +198,4 @@ if wishing to use the MSVC debugger with Rust in VS code, which may be preferred
     }
     ```
 
-3. Add `"debug.allowBreakpointsEverywhere": true` to VS code settings.json
+3. Add `"debug.allowBreakpointsEverywhere": true` to VS code settings.json

Makefile.toml

@@ -3,9 +3,9 @@ default_to_workspace = false
 
 [env]
 # Determines the version of Elasticsearch docker container used
-STACK_VERSION = "7.x-SNAPSHOT"
+STACK_VERSION = { value = "7.x-SNAPSHOT", condition = { env_not_set = ["STACK_VERSION"] }}
 # Determines the distribution of docker container used. Either platinum or free
-TEST_SUITE = "platinum"
+TEST_SUITE = { value = "free", condition = { env_not_set = ["TEST_SUITE"] }}
 # Set publish flags to dry-run by default, to force user to explicitly define for publishing
 CARGO_MAKE_CARGO_PUBLISH_FLAGS = "--dry-run"
 

yaml_test_runner/src/generator.rs

@@ -36,7 +36,7 @@ use yaml_rust::{Yaml, YamlLoader};
 /// The test suite to compile
 #[derive(Debug, PartialEq)]
 pub enum TestSuite {
-    Oss,
+    Free,
     XPack,
 }
 
@@ -121,7 +121,7 @@ impl<'a> YamlTests<'a> {
         let (setup_fn, setup_call) = Self::generate_fixture(&self.setup);
         let (teardown_fn, teardown_call) = Self::generate_fixture(&self.teardown);
         let general_setup_call = match self.suite {
-            TestSuite::Oss => quote!(client::general_oss_setup().await?;),
+            TestSuite::Free => quote!(client::general_oss_setup().await?;),
             TestSuite::XPack => quote!(client::general_xpack_setup().await?;),
         };
 
@@ -406,9 +406,9 @@ pub fn generate_tests_from_yaml(
                         }
 
                         match top_dir.as_str() {
-                            "oss" => TestSuite::Oss,
+                            "free" => TestSuite::Free,
                             "xpack" => TestSuite::XPack,
-                            _ => panic!("Unknown test suite"),
+                            _ => panic!("Unknown test suite {:?}", path),
                         }
                     };
 

yaml_test_runner/src/github.rs

@@ -67,7 +67,7 @@ pub fn download_test_suites(branch: &str, download_dir: &PathBuf) -> Result<(),
         let file = entry?;
         let path = file.path()?;
         if oss_test.is_match(&path) {
-            write_test_file(download_dir, "oss", file)?;
+            write_test_file(download_dir, "free", file)?;
         } else if xpack_test.is_match(&path) {
             write_test_file(download_dir, "xpack", file)?;
         }

yaml_test_runner/src/main.rs

@@ -148,13 +148,14 @@ fn branch_suite_and_version_from_elasticsearch(
         .danger_accept_invalid_certs(true)
         .build()?;
 
+    let suite = match std::env::var("TEST_SUITE") {
+        Err(_) => panic!("Env var TEST_SUITE is not defined"),
+        Ok(ref s) if s == "free" => TestSuite::Free,
+        _ => TestSuite::XPack,
+    };
     let mut response = client.get(url).send()?;
     let json: Value = response.json()?;
     let branch = json["version"]["build_hash"].as_str().unwrap().to_string();
-    let suite = match json["version"]["build_flavor"].as_str().unwrap() {
-        "oss" => TestSuite::Oss,
-        _ => TestSuite::XPack,
-    };
 
     // any prerelease part needs to be trimmed because the semver crate only allows
     // a version with a prerelease to match against predicates, if at least one predicate

yaml_test_runner/src/step/do.rs

@@ -38,7 +38,7 @@ impl ToTokens for Catch {
     fn to_tokens(&self, tokens: &mut Tokens) {
         fn http_status_code(status_code: u16, tokens: &mut Tokens) {
             tokens.append(quote! {
-                assert_status_code!(response.status_code(), #status_code);
+                assert_response_status_code!(response, #status_code);
             });
         }
 

yaml_test_runner/tests/common/client.rs

@@ -247,6 +247,35 @@ async fn wait_for_yellow_status(client: &Elasticsearch) -> Result<(), Error> {
 }
 
 async fn delete_indices(client: &Elasticsearch) -> Result<(), Error> {
+    // Hand-crafted request as the indices.delete_data_stream spec doesn't yet have the
+    // "expand_wildcards" parameter that is needed to delete ILM data streams
+    //
+    // Not deleting data streams yields errors like this when trying to delete hidden indices:
+    // {
+    //   "type":"illegal_argument_exception"
+    //   "reason":"index [.ds-ilm-history-5-2021.02.14-000001] is the write index for data
+    //             stream [ilm-history-5] and cannot be deleted"
+    // }
+    //
+    // Quoting the docs:
+    // You cannot delete the current write index of a data stream. To delete the index,
+    // you must roll over the data stream so a new write index is created. You can then use
+    // the delete index API to delete the previous write index.
+    //
+    let delete_response = client
+        .transport()
+        .send(
+            Method::Delete,
+            "/_data_stream/*",
+            elasticsearch::http::headers::HeaderMap::new(),
+            Some(&[("expand_wildcards", "hidden")]),
+            None::<()>, // body
+            None,       // timeout
+        )
+        .await?;
+
+    assert_response_success!(delete_response);
+
     let delete_response = client
         .indices()
         .delete(IndicesDeleteParts::Index(&["*"]))

yaml_test_runner/tests/common/macros.rs

@@ -17,27 +17,59 @@
  * under the License.
  */
 /// Asserts that a [Response] has a status code >=200 and <300
+
 #[macro_export]
 macro_rules! assert_response_success {
     ($response:ident) => {{
-        assert!(
-            $response.status_code().is_success(),
-            "expected response to be successful but was {}",
-            $response.status_code().as_u16()
-        );
+        let status_code = $response.status_code();
+        if !status_code.is_success() {
+            let text = $response
+                .text()
+                .await
+                .unwrap_or("[no response]".to_string());
+            panic!(
+                "expected response to be successful but was {}. Response: {}",
+                status_code, text
+            );
+        }
     }};
 }
 
 /// Asserts that a [Response] has a status code >=200 and <300 or matches the passed status
 #[macro_export]
 macro_rules! assert_response_success_or {
     ($response:ident, $status:expr) => {{
-        assert!(
-            $response.status_code().is_success() || $response.status_code().as_u16() == $status,
-            "expected response to be successful or {} but was {}",
-            $status,
-            $response.status_code().as_u16()
-        );
+        let status_code = $response.status_code();
+        if !status_code.is_success() && status_code.as_u16() != $status {
+            let text = $response
+                .text()
+                .await
+                .unwrap_or("[no response]".to_string());
+            panic!(
+                "expected response to be successful or {} but was {}",
+                $status,
+                status_code.as_u16()
+            );
+        }
+    }};
+}
+
+/// Asserts that a [Response] has a status that matches the passed status
+#[macro_export]
+macro_rules! assert_response_status_code {
+    ($response:ident, $status:expr) => {{
+        let status_code = $response.status_code();
+        if status_code.as_u16() != $status {
+            let text = $response
+                .text()
+                .await
+                .unwrap_or("[no response]".to_string());
+            panic!(
+                "expected response to be {} but was {}",
+                $status,
+                status_code.as_u16()
+            );
+        }
     }};
 }