Skip to content
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.

Sign up for GitHub

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

Resolve flaky test harness tests in command and integration suites #2482

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Resolve flaky test harness tests in command and integration suites #2482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions tremor-cli/src/test/assert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ pub(crate) struct FileBasedAssert {
pub(crate) contains: Option<Vec<String>>,
pub(crate) doesnt_contain: Option<Vec<String>>,
pub(crate) equals_file: Option<String>,
pub(crate) is_empty: Option<bool>,
pub(crate) is_not_empty: Option<bool>,
}

pub(crate) type Asserts = Vec<FileBasedAssert>;
Expand Down Expand Up @@ -329,6 +331,7 @@ fn process_equals_file(
})
}

#[allow(clippy::too_many_lines)]
pub(crate) fn process_filebased_asserts(
prefix: &str,
stdout_path: &Path,
Expand All @@ -345,6 +348,8 @@ pub(crate) fn process_filebased_asserts(
contains: None,
doesnt_contain: None,
equals_file: None,
is_empty: None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_empty and is_not_empty seem to be the same just inverted. It looks like they can be reduced to a single flag, halfing the needed code and preventing possible confusion.

is_not_empty: None,
..
} => {
stats.skip();
Expand All @@ -355,6 +360,8 @@ pub(crate) fn process_filebased_asserts(
contains,
doesnt_contain,
equals_file,
is_empty,
is_not_empty,
..
} => {
let file = match source {
Expand Down Expand Up @@ -394,9 +401,79 @@ pub(crate) fn process_filebased_asserts(
&mut counter,
)?);
}

if let Some(is_empty) = is_empty {
stats.assert();
counter += 1;
let condition = (std::fs::metadata(&file)?.len() == 0) == *is_empty;
stats.report(condition, &file);
status::assert_has(
prefix,
&format!("Assert {counter}"),
&format!(" File `{}` is empty", &file),
None,
condition,
)?;
elements.push(report::TestElement {
description: format!("File `{file}` is empty"),
info: None,
hidden: false,
keyword: report::KeywordKind::Predicate,
result: report::ResultKind {
status: if condition {
report::StatusKind::Passed
} else {
report::StatusKind::Failed
},
duration: 0,
},
});
}

if let Some(is_not_empty) = is_not_empty {
stats.assert();
counter += 1;
let condition = (std::fs::metadata(&file)?.len() != 0) == *is_not_empty;
stats.report(condition, &file);
status::assert_has(
prefix,
&format!("Assert {counter}"),
&format!(" File `{}` is not empty", &file),
None,
condition,
)?;
elements.push(report::TestElement {
description: format!("File `{file}` is not empty"),
info: None,
hidden: false,
keyword: report::KeywordKind::Predicate,
result: report::ResultKind {
status: if condition {
report::StatusKind::Passed
} else {
report::StatusKind::Failed
},
duration: 0,
},
});
}
}
}
}

Ok((stats, elements))
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_contains() -> Result<()> {
let file = "src/test/assert/contains.txt";
let contains = vec!["snot".to_string(), "badger".to_string()];
let result = file_contains(file, &contains, None)?;
assert!(result);
Ok(())
}
}
2 changes: 2 additions & 0 deletions tremor-cli/src/test/assert/contains.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
snot
badger
11 changes: 11 additions & 0 deletions tremor-cli/tests/cli/command.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ suites:
- "Error:"
- "3 | emit event.baz"
- " | ^^^ Trying to access a non existing event key `baz`\n\n"
- name: setup for tremor new creates
command: rm -rf test
tags:
- run
- new
status: 0
expects:
- source: stdout
is_empty: true
- source: stderr
is_empty: true
- name: tremor new creates
command: tremor new test
tags:
Expand Down
54 changes: 51 additions & 3 deletions tremor-cli/tests/integration/elastic-verify-gd/assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,58 @@ status: 0
name: elastic-verify-gd
asserts:
- source: err.log
equals_file: expected_err.json
contains:
- |
{"action":"index","correlation":"tons","index":"my_little_index","payload":{"action":null,"cause":"this one has a different type underneath the same key and thus will fail elastic","snot":"tons","tremor":12},"success":false}
- source: err.log
contains:
- |
{"action":"index","correlation":"baz","index":"my_little_index","payload":{"action":null,"cause":"this one has a different type underneath the same key and thus will fail elastic","snot":"baz","tremor":12},"success":false}
- source: ok.log
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a single contains with multiple elements? (same for error.log)

contains:
- |
{"action":"index","correlation":"badger","index":"my_little_index","payload":{"action":null,"snot":"badger"},"success":true}
- source: ok.log
contains:
- |
{"action":"index","correlation":"foo","index":"my_little_index","payload":{"action":null,"snot":"foo"},"success":true}
- source: ok.log
contains:
- |
{"action":"index","correlation":"racoon","index":"my_little_index","payload":{"action":null,"snot":"racoon","tremor":[true,true,true,false]},"success":true}
- source: ok.log
contains:
- |
{"action":"index","correlation":"bar","index":"my_little_index","payload":{"action":null,"snot":"bar","tremor":[true,true,true,false]},"success":true}
- source: ok.log
contains:
- |
{"action":"delete","correlation":"badger","index":"my_little_index","payload":{"action":"delete","doc_id":"badger","snot":"badger"},"success":true}
- source: ok.log
contains:
- |
{"action":"delete","correlation":"fop","index":"my_little_index","payload":{"action":"delete","doc_id":"badger","snot":"fop"},"success":true}
- source: ok.log
contains:
- |
{"action":"index","correlation":"badger","index":"my_little_index","payload":{"action":"index","doc_id":"badger","snot":"badger"},"success":true}
- source: ok.log
contains:
- |
{"action":"index","correlation":"snoop","index":"my_little_index","payload":{"action":"index","doc_id":"badger","snot":"snoop"},"success":true}
- source: ok.log
contains:
- |
{"action":"update","correlation":"badger","index":"my_little_index","payload":{"action":"update","doc_id":"badger","snot":"badger"},"success":true}
- source: ok.log
equals_file: expected_ok.json
contains:
- |
{"action":"update","correlation":"dogg","index":"my_little_index","payload":{"action":"update","doc_id":"badger","snot":"dogg"},"success":true}
- source: ok.log
contains:
- |
{"action":"index","correlation":"badger","index":"my_little_index","payload":{"action":null,"snot":"badger"},"success":true}
- source: fg.err.log
contains:
- |
All required CB events received.
All required CB events received.
Loading