Skip to content

Commit

Permalink
feat: Remove test statements (#4910)
Browse files Browse the repository at this point in the history
* feat: Remove test statements

With all `test` statements being converted to `testcase` and neither being used outside of
testing flux (right? no one else is using either?). We can safely remove this old way of writing tests.

Closes #4768

* feat: Remove testing.benchmark/inspect/run

These seem to depend on the `test` statement so without it, they have no purpose.

* chore: make generate
  • Loading branch information
Markus Westerlind authored Jun 27, 2022
1 parent 0bd05dd commit 02f463f
Show file tree
Hide file tree
Showing 19 changed files with 16 additions and 509 deletions.
16 changes: 0 additions & 16 deletions libflux/flux-core/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,6 @@ pub enum Statement {
Return(Box<ReturnStmt>),
#[serde(rename = "BadStatement")]
Bad(Box<BadStmt>),
#[serde(rename = "TestStatement")]
Test(Box<TestStmt>),
#[serde(rename = "TestCaseStatement")]
TestCase(Box<TestCaseStmt>),
#[serde(rename = "BuiltinStatement")]
Expand All @@ -259,7 +257,6 @@ impl Statement {
Statement::Option(wrapped) => &wrapped.base,
Statement::Return(wrapped) => &wrapped.base,
Statement::Bad(wrapped) => &wrapped.base,
Statement::Test(wrapped) => &wrapped.base,
Statement::TestCase(wrapped) => &wrapped.base,
Statement::Builtin(wrapped) => &wrapped.base,
}
Expand All @@ -273,7 +270,6 @@ impl Statement {
Statement::Option(_) => 2,
Statement::Return(_) => 3,
Statement::Bad(_) => 4,
Statement::Test(_) => 5,
Statement::TestCase(_) => 7,
Statement::Builtin(_) => 6,
}
Expand All @@ -286,7 +282,6 @@ impl Statement {
Statement::Option(_) => "option",
Statement::Return(_) => "return",
Statement::Bad(_) => "bad",
Statement::Test(_) => "test",
Statement::TestCase(_) => "testcase",
Statement::Builtin(_) => "builtin",
}
Expand Down Expand Up @@ -804,17 +799,6 @@ pub struct PropertyType {
pub monotype: MonoType,
}

/// TestStmt declares a Flux test case.
#[allow(missing_docs)]
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
pub struct TestStmt {
#[serde(skip_serializing_if = "BaseNode::is_empty")]
#[serde(default)]
#[serde(flatten)]
pub base: BaseNode,
pub assignment: VariableAssgn,
}

/// Declares a Flux test case.
// XXX: rockstar (17 Nov 2020) - This should replace the TestStmt above, once
// it has been extended enough to cover the existing use cases.
Expand Down
54 changes: 0 additions & 54 deletions libflux/flux-core/src/ast/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -993,60 +993,6 @@ fn test_json_builtin_statement_comments() {
want: `{"type":"TestStatement","assignment":{"type":"VariableAssignment","id":{"type":"Identifier","name":"mean"},"init":{"type":"ObjectExpression","properties":[{"type":"Property","key":{"type":"Identifier","name":"want"},"value":{"type":"IntegerLiteral","value":"0"}},{"type":"Property","key":{"type":"Identifier","name":"got"},"value":{"type":"IntegerLiteral","value":"0"}}]}}}`,
},
*/
#[test]
fn test_json_test_statement() {
let n = Statement::Test(Box::new(TestStmt {
base: BaseNode::default(),
assignment: VariableAssgn {
base: BaseNode::default(),
id: Identifier {
base: BaseNode::default(),
name: "mean".to_string(),
},
init: Expression::Object(Box::new(ObjectExpr {
base: BaseNode::default(),
lbrace: vec![],
with: None,
properties: vec![
Property {
base: BaseNode::default(),
key: PropertyKey::Identifier(Identifier {
base: BaseNode::default(),
name: "want".to_string(),
}),
separator: vec![],
value: Some(Expression::Integer(IntegerLit {
base: Default::default(),
value: 0,
})),
comma: vec![],
},
Property {
base: BaseNode::default(),
key: PropertyKey::Identifier(Identifier {
base: BaseNode::default(),
name: "got".to_string(),
}),
separator: vec![],
value: Some(Expression::Integer(IntegerLit {
base: Default::default(),
value: 0,
})),
comma: vec![],
},
],
rbrace: vec![],
})),
},
}));
let serialized = serde_json::to_string(&n).unwrap();
assert_eq!(
serialized,
r#"{"type":"TestStatement","assignment":{"id":{"name":"mean"},"init":{"type":"ObjectExpression","properties":[{"type":"Property","key":{"type":"Identifier","name":"want"},"value":{"type":"IntegerLiteral","value":"0"}},{"type":"Property","key":{"type":"Identifier","name":"got"},"value":{"type":"IntegerLiteral","value":"0"}}]}}}"#
);
let deserialized: Statement = serde_json::from_str(serialized.as_str()).unwrap();
assert_eq!(deserialized, n)
}

#[test]
fn test_json_test_case_statement() {
Expand Down
7 changes: 0 additions & 7 deletions libflux/flux-core/src/ast/walk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ pub enum Node<'a> {
ReturnStmt(&'a ReturnStmt),
#[display(fmt = "BadStmt")]
BadStmt(&'a BadStmt),
#[display(fmt = "TestStmt")]
TestStmt(&'a TestStmt),
#[display(fmt = "TestCaseStmt")]
TestCaseStmt(&'a TestCaseStmt),
#[display(fmt = "BuiltinStmt")]
Expand Down Expand Up @@ -160,7 +158,6 @@ impl<'a> Node<'a> {
Node::OptionStmt(n) => &n.base,
Node::ReturnStmt(n) => &n.base,
Node::BadStmt(n) => &n.base,
Node::TestStmt(n) => &n.base,
Node::TestCaseStmt(n) => &n.base,
Node::BuiltinStmt(n) => &n.base,
Node::Block(n) => &n.base,
Expand Down Expand Up @@ -217,7 +214,6 @@ impl<'a> Node<'a> {
Statement::Option(s) => Node::OptionStmt(s),
Statement::Return(s) => Node::ReturnStmt(s),
Statement::Bad(s) => Node::BadStmt(s),
Statement::Test(s) => Node::TestStmt(s),
Statement::TestCase(s) => Node::TestCaseStmt(s),
Statement::Builtin(s) => Node::BuiltinStmt(s),
}
Expand Down Expand Up @@ -396,9 +392,6 @@ where
walk(v, Node::from_expr(&n.argument));
}
Node::BadStmt(_) => {}
Node::TestStmt(n) => {
walk(v, Node::VariableAssgn(&n.assignment));
}
Node::TestCaseStmt(n) => {
walk(v, Node::Identifier(&n.id));
walk(v, Node::Block(&n.block));
Expand Down
13 changes: 0 additions & 13 deletions libflux/flux-core/src/ast/walk/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,19 +263,6 @@ fn test_return_stmt() {
)
}
#[test]
fn test_test_stmt() {
test_walk(
"test a = 1",
vec![
"File",
"TestStmt",
"VariableAssgn",
"Identifier",
"IntegerLit",
],
)
}
#[test]
fn test_builtin_stmt() {
test_walk(
"builtin a",
Expand Down
9 changes: 0 additions & 9 deletions libflux/flux-core/src/formatter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,14 +676,6 @@ impl<'doc> Formatter<'doc> {
self.err = Some(anyhow!("bad statement"));
arena.nil()
}
Statement::Test(n) => {
docs![
arena,
self.format_comments(&n.base.comments),
"test ",
self.format_variable_assignment(&n.assignment),
]
}
Statement::TestCase(n) => {
let comment = self.format_comments(&n.base.comments);
let prefix = docs![
Expand Down Expand Up @@ -1796,7 +1788,6 @@ fn starts_with_comment(n: Node) -> bool {
Node::OptionStmt(n) => !n.base.comments.is_empty(),
Node::ReturnStmt(n) => !n.base.comments.is_empty(),
Node::BadStmt(_) => false,
Node::TestStmt(n) => !n.base.comments.is_empty(),
Node::TestCaseStmt(n) => !n.base.comments.is_empty(),
Node::BuiltinStmt(n) => !n.base.comments.is_empty(),
Node::Block(n) => !n.lbrace.is_empty(),
Expand Down
13 changes: 4 additions & 9 deletions libflux/flux-core/src/formatter/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,6 @@ fn comments2() {
);
assert_unchanged("f = (a=1, b=2) =>\n //comment\n a()");

assert_unchanged("//comment\ntest a = 1");
assert_unchanged("test //comment\na = 1");
assert_unchanged("test a\n //comment\n = 1");
assert_unchanged("test a =\n //comment\n 1");

assert_unchanged("//comment\nreturn x");
assert_unchanged("return\n //comment\n x");

Expand Down Expand Up @@ -1092,11 +1087,11 @@ test2 = 2",
#[test]
fn preserve_multiline_test() {
// ensure functions given preserve their structure
//assert_unchanged("test _convariance_missing_column_2 = () =>
//assert_unchanged("_convariance_missing_column_2 = () =>
//({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: covariance_missing_column_2})");

assert_unchanged(
"test _covariance_missing_column_2 = () =>
"_covariance_missing_column_2 = () =>
({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: covariance_missing_column_2})",
);

Expand Down Expand Up @@ -1414,7 +1409,7 @@ diff =
|> range(start: 2020-04-27T00:00:00Z, stop: 2020-05-01T00:00:00Z)
|> anomalydetection.mad(threshold: 3.0)
test _mad = () => ({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: t_mad})"#,
_mad = () => ({input: testing.loadStorage(csv: inData), want: testing.loadMem(csv: outData), fn: t_mad})"#,
);

// comments
Expand Down Expand Up @@ -1460,7 +1455,7 @@ diff =
);

assert_unchanged(
r#"test _join_panic = () =>
r#"_join_panic = () =>
// to trigger the panic, switch the testing.loadStorage() csv from `passData` to `failData`
({input: testing.loadStorage(csv: passData), want: testing.loadMem(csv: outData), fn: t_join_panic})"#,
);
Expand Down
16 changes: 0 additions & 16 deletions libflux/flux-core/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,6 @@ impl<'input> Parser<'input> {
TokenType::Ident => self.parse_ident_statement(),
TokenType::Option => self.parse_option_assignment(),
TokenType::Builtin => self.parse_builtin_statement(),
TokenType::Test => self.parse_test_statement(),
TokenType::TestCase => self.parse_testcase_statement(),
TokenType::Return => self.parse_return_statement(),
_ => {
Expand Down Expand Up @@ -812,21 +811,6 @@ impl<'input> Parser<'input> {
}
}

fn parse_test_statement(&mut self) -> Statement {
let t = self.expect(TokenType::Test);
let id = self.parse_identifier();
let assign = self.peek().clone();
let assignment = self.parse_assign_statement();
Statement::Test(Box::new(TestStmt {
base: self.base_node_from_other_end_c(&t, assignment.base(), &t),
assignment: VariableAssgn {
base: self.base_node_from_others_c(&id.base, assignment.base(), &assign),
id,
init: assignment,
},
}))
}

fn parse_testcase_statement(&mut self) -> Statement {
let t = self.expect(TokenType::TestCase);
let id = self.parse_identifier();
Expand Down
95 changes: 0 additions & 95 deletions libflux/flux-core/src/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1450,101 +1450,6 @@ fn builtin() {
)
}

#[test]
fn test_statement() {
let mut p = Parser::new(r#"test mean = {want: 0, got: 0}"#);
let parsed = p.parse_file("".to_string());
let loc = Locator::new(&p.source[..]);
assert_eq!(
parsed,
File {
base: BaseNode {
location: loc.get(1, 1, 1, 30),
..BaseNode::default()
},
name: "".to_string(),
metadata: "parser-type=rust".to_string(),
package: None,
imports: vec![],
body: vec![Statement::Test(Box::new(TestStmt {
base: BaseNode {
location: loc.get(1, 1, 1, 30),
..BaseNode::default()
},
assignment: VariableAssgn {
base: BaseNode {
location: loc.get(1, 6, 1, 30),
..BaseNode::default()
},
id: Identifier {
base: BaseNode {
location: loc.get(1, 6, 1, 10),
..BaseNode::default()
},
name: "mean".to_string()
},
init: Expression::Object(Box::new(ObjectExpr {
base: BaseNode {
location: loc.get(1, 13, 1, 30),
..BaseNode::default()
},
lbrace: vec![],
with: None,
properties: vec![
Property {
base: BaseNode {
location: loc.get(1, 14, 1, 21),
..BaseNode::default()
},
key: PropertyKey::Identifier(Identifier {
base: BaseNode {
location: loc.get(1, 14, 1, 18),
..BaseNode::default()
},
name: "want".to_string()
}),
separator: vec![],
value: Some(Expression::Integer(IntegerLit {
base: BaseNode {
location: loc.get(1, 20, 1, 21),
..BaseNode::default()
},
value: 0
})),
comma: vec![],
},
Property {
base: BaseNode {
location: loc.get(1, 23, 1, 29),
..BaseNode::default()
},
key: PropertyKey::Identifier(Identifier {
base: BaseNode {
location: loc.get(1, 23, 1, 26),
..BaseNode::default()
},
name: "got".to_string()
}),
separator: vec![],
value: Some(Expression::Integer(IntegerLit {
base: BaseNode {
location: loc.get(1, 28, 1, 29),
..BaseNode::default()
},
value: 0
})),
comma: vec![],
}
],
rbrace: vec![],
}))
}
}))],
eof: vec![],
},
)
}

#[test]
fn comment() {
let mut p = Parser::new(
Expand Down
8 changes: 0 additions & 8 deletions libflux/flux-core/src/semantic/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,6 @@ impl<'a> Converter<'a> {
ast::Statement::Builtin(s) => {
Statement::Builtin(self.convert_builtin_statement(package, s)?)
}
ast::Statement::Test(s) => Statement::Test(Box::new(self.convert_test_statement(s))),
ast::Statement::TestCase(s) => {
Statement::TestCase(Box::new(self.convert_testcase(package, s)))
}
Expand Down Expand Up @@ -734,13 +733,6 @@ impl<'a> Converter<'a> {
types::PolyType { vars, cons, expr }
}

fn convert_test_statement(&mut self, stmt: &ast::TestStmt) -> TestStmt {
TestStmt {
loc: stmt.base.location.clone(),
assignment: self.convert_variable_assignment(None, &stmt.assignment),
}
}

fn convert_expression_statement(&mut self, stmt: &ast::ExprStmt) -> ExprStmt {
ExprStmt {
loc: stmt.base.location.clone(),
Expand Down
Loading

0 comments on commit 02f463f

Please sign in to comment.