Split up tests #191
Split up tests #191
Conversation
|
Ok, new One example good to look would be test_case!(rename, async move {
let test_cases = vec![
("CREATE TABLE Foo (id INTEGER);", Ok(Payload::Create)),
(
"INSERT INTO Foo VALUES (1), (2), (3);",
Ok(Payload::Insert(3)),
),
("SELECT id FROM Foo", Ok(select!(id; I64; 1; 2; 3))),
(
"ALTER TABLE Foo2 RENAME TO Bar;",
Err(AlterTableError::TableNotFound("Foo2".to_owned()).into()),
),
("ALTER TABLE Foo RENAME TO Bar;", Ok(Payload::AlterTable)),
("SELECT id FROM Bar", Ok(select!(id; I64; 1; 2; 3))),
(
"ALTER TABLE Bar RENAME COLUMN id TO new_id",
Ok(Payload::AlterTable),
),
("SELECT new_id FROM Bar", Ok(select!(new_id; I64; 1; 2; 3))),
(
"ALTER TABLE Bar RENAME COLUMN hello TO idid",
Err(AlterTableError::RenamingColumnNotFound.into()),
),
];
for (sql, expected) in test_cases.into_iter() {
test!(expected, sql);
}
}); This actually contains two independent tests.
Therefore, it might be better with separated ones. test_case!(rename_table, async move {
let test_cases = vec![
("CREATE TABLE Foo (id INTEGER);", Ok(Payload::Create)),
(
"INSERT INTO Foo VALUES (1), (2), (3);",
Ok(Payload::Insert(3)),
),
(
"ALTER TABLE Foo2 RENAME TO Bar;",
Err(AlterTableError::TableNotFound("Foo2".to_owned()).into()),
),
("ALTER TABLE Foo RENAME TO Bar;", Ok(Payload::AlterTable)),
("SELECT id FROM Bar", Ok(select!(id; I64; 1; 2; 3))),
];
for (sql, expected) in test_cases.into_iter() {
test!(expected, sql);
}
});
test_case!(rename_column, async move {
let test_cases = vec![
("CREATE TABLE Foo (id INTEGER);", Ok(Payload::Create)),
(
"INSERT INTO Foo VALUES (1), (2), (3);",
Ok(Payload::Insert(3)),
),
(
"ALTER TABLE Foo RENAME COLUMN id TO new_id",
Ok(Payload::AlterTable),
),
("SELECT new_id FROM Foo", Ok(select!(new_id; I64; 1; 2; 3))),
(
"ALTER TABLE Foo RENAME COLUMN hello TO idid",
Err(AlterTableError::RenamingColumnNotFound.into()),
),
];
for (sql, expected) in test_cases.into_iter() {
test!(expected, sql);
}
});And then, if we even make an another helper macro to omit this kind of repetition codes (below), codes lines will even become less. for (sql, expected) in test_cases.into_iter() {
test!(expected, sql);
} I don't think it is good to have two files which each one has only around 10 lines of codes. |
Could you elaborate on this? |
|
@KyGost ok, I think that is more efficient because we can scan the whole file without tab switching or scrolling, and these two files share the same parent category, alter table. It is basically similar behavior with what we do on Rust codes. |
|
Hmmm... I tend to like splitting whenever there's a difference in context, including for code. I think us using different editors might contribute to this. I've just realised-- can't we just do: mod rename_table {
test_case!(async move {
let test_cases = vec![
("CREATE TABLE Foo (id INTEGER);", Ok(Payload::Create)),
(
"INSERT INTO Foo VALUES (1), (2), (3);",
Ok(Payload::Insert(3)),
),
(
"ALTER TABLE Foo2 RENAME TO Bar;",
Err(AlterTableError::TableNotFound("Foo2".to_owned()).into()),
),
("ALTER TABLE Foo RENAME TO Bar;", Ok(Payload::AlterTable)),
("SELECT id FROM Bar", Ok(select!(id; I64; 1; 2; 3))),
];
for (sql, expected) in test_cases.into_iter() {
test!(expected, sql);
}
});
}
mod rename_column {
test_case!(async move {
let test_cases = vec![
("CREATE TABLE Foo (id INTEGER);", Ok(Payload::Create)),
(
"INSERT INTO Foo VALUES (1), (2), (3);",
Ok(Payload::Insert(3)),
),
(
"ALTER TABLE Foo RENAME COLUMN id TO new_id",
Ok(Payload::AlterTable),
),
("SELECT new_id FROM Foo", Ok(select!(new_id; I64; 1; 2; 3))),
(
"ALTER TABLE Foo RENAME COLUMN hello TO idid",
Err(AlterTableError::RenamingColumnNotFound.into()),
),
];
for (sql, expected) in test_cases.into_iter() {
test!(expected, sql);
}
});
} |
I also agree with splitting for different context, but it doesn't need to be as a file. but, what is better than before? before test_case!(rename_table, async move { .. });new mod rename_table {
test_case!(async move { .. })
} |
I don't think it's as simple as expansion, splitting files makes it a lot easier to locate where each thing is. I digress however; this is not an important point.
What is better is that the files are much easier to manage, we use the |
|
Ok I use vim and too many tabs are quite bothering, harder to move and I cannot read the full tab names. mod rename_table {
test_case!(async move { .. })
}Ok the point was, if you intend to use |
The wrap with module is because normally it they don't have seperate names (anymore), seperate function names is a bit tedious. |
mod rename_table {
test_case!(async move { .. })
}this will become mod rename_table {
pub async fn rename_table( .. ) { .. }
}original one simply generates pub async fn rename_table( .. ) { .. }This is not the matter of preference, there's really no meaning of module in this case. If the file module name is |
With the code, as it is now, it would become: mod rename_table {
pub async fn test( .. ) { .. }
}as stated I removed the individual function names. Saying that, however, I'm considering bringing them back for secondary functions. |
mod rename_table {
pub async fn test( .. ) { .. }
}ok, it's better than having mod rename_table {
pub async fn test( .. ) { .. }
pub async fn test_something_different( .. ) { .. }
pub async fn test_something_another( .. ) { .. }
}In this above example, it makes sense to have a module because it contains multiple different methods. The reason I mentioned there's no meaning is because |
|
nb. above was discussed in a voice chat. |
|
Ok, please give me some time to show what I plan to make the current test system better, I think it would be better with using real implementation codes. |
|
Closing PRs which are merged and working well in the fork. |
This PR makes it easier to have foldered tests by adding the
build_suite!()macro.