Refactor testing modules #1491
Refactor testing modules #1491
Conversation
datafusion/tests/mod.rs
Outdated
| @@ -0,0 +1 @@ | |||
| pub mod sql; | |||
There was a problem hiding this comment.
Maybe we don't need to public this mod
| pub(crate) use test_expression; | ||
| pub mod aggregates; | ||
| #[cfg(feature = "avro")] | ||
| pub mod avro; |
There was a problem hiding this comment.
The issue like above.
If the mod is used in this model, we don't need to public them.
| @@ -0,0 +1,187 @@ | |||
| use super::*; | |||
There was a problem hiding this comment.
We should add the following to each new file.
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
There was a problem hiding this comment.
I have to add these to all the new files, I'll start a list
|
Nice work! thanks, @hntd187, our tests become more clear. |
|
I noticed (I don't remember who) pointed out I was missing a test. That comment isn't here anymore, but I went back and compared the test list from the old |
It's me. After checking carefully, I find you did not miss the test. |
I suggest that we do the test rename / modification as part of a different PR where we can focus on those changes and they don't get lost in the code movement |
There was a problem hiding this comment.
Thank you @hntd187 -- I didn't review this one line by line, but I trust that @liukun4515 and @xudong963 did -- thank you all
Is this one ready to merge in? It is still marked as a draft, but it seems to be
|
Thanks again @hntd187 @xudong963 and @liukun4515 |
Which issue does this PR close?
Re #743
Closes #.
Rationale for this change
Re-organize tests to be more isolated and clearer to new tests being written.
What changes are included in this PR?
Lots of copy pasting and file re-naming.
Are there any user-facing changes?
No.
Outstanding Work:
pubon most of the mods