alaahong commented on code in PR #716: URL: https://github.com/apache/fesod/pull/716#discussion_r2596133259
########## fesod/src/test/java/org/apache/fesod/sheet/model/ComplexData.java: ########## @@ -0,0 +1,105 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.model; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Date; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import org.apache.fesod.sheet.annotation.ExcelProperty; + +/** + * Complex data model for testing various data type conversions. + * Consolidated from converter tests. + */ +@Getter +@Setter +@EqualsAndHashCode +public class ComplexData { + @ExcelProperty("日期") + private Date date; + + @ExcelProperty("本地日期") + private LocalDate localDate; + + @ExcelProperty("本地日期时间") + private LocalDateTime localDateTime; + + @ExcelProperty("布尔") + private Boolean booleanData; + + @ExcelProperty("大数") + private BigDecimal bigDecimal; + + @ExcelProperty("大整数") + private BigInteger bigInteger; + + @ExcelProperty("长整型") + private long longData; Review Comment: Why is not the wrapper class？ ########## fesod/pom.xml: ########## @@ -157,6 +157,12 @@ <artifactId>jazzer-junit</artifactId> <scope>test</scope> </dependency> + <dependency> + <groupId>org.assertj</groupId> + <artifactId>assertj-core</artifactId> + <version>3.24.2</version> Review Comment: please manage version with parent dependency management ########## fesod/src/test/java/org/apache/fesod/sheet/model/SimpleData.java: ########## @@ -0,0 +1,65 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.model; + +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import org.apache.fesod.sheet.annotation.ExcelProperty; + +/** + * Simple data model for basic read/write operations. + * Consolidated from various feature-specific packages. + */ +@Getter +@Setter +@EqualsAndHashCode +public class SimpleData { + @ExcelProperty("姓名") + private String name; + + @ExcelProperty("数字") + private Double number; + + @ExcelProperty("整数") + private Integer integer; + + public SimpleData() {} + + public SimpleData(String name) { Review Comment: Why mix Lombok and hard code for bean usage code? ########## fesod/src/test/java/org/apache/fesod/sheet/model/ComplexData.java: ########## @@ -0,0 +1,105 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.model; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Date; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import org.apache.fesod.sheet.annotation.ExcelProperty; + +/** + * Complex data model for testing various data type conversions. + * Consolidated from converter tests. + */ +@Getter +@Setter +@EqualsAndHashCode +public class ComplexData { + @ExcelProperty("日期") + private Date date; + + @ExcelProperty("本地日期") + private LocalDate localDate; + + @ExcelProperty("本地日期时间") + private LocalDateTime localDateTime; + + @ExcelProperty("布尔") + private Boolean booleanData; + + @ExcelProperty("大数") Review Comment: Decimal~ ########## fesod/src/test/java/org/apache/fesod/sheet/model/ComplexData.java: ########## @@ -0,0 +1,105 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.model; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Date; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import org.apache.fesod.sheet.annotation.ExcelProperty; + +/** + * Complex data model for testing various data type conversions. + * Consolidated from converter tests. + */ +@Getter +@Setter +@EqualsAndHashCode +public class ComplexData { Review Comment: By default, it's better to provide English header in code. You can create another class with Chinese prefix. ########## fesod/src/test/java/org/apache/fesod/sheet/testkit/base/AbstractExcelTest.java: ########## @@ -0,0 +1,153 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.testkit.base; + +import java.io.File; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Stream; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.testkit.enums.ExcelFormat; +import org.apache.fesod.sheet.testkit.listeners.CollectingReadListener; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.provider.Arguments; + +/** + * Abstract base class for Excel tests providing common infrastructure. + * + * <p>Features:</p> + * <ul> + * <li>Automatic temp directory management via JUnit 5 @TempDir</li> + * <li>Format provider methods for parameterized tests</li> + * <li>Common read/write operations</li> + * <li>Format-aware file creation</li> + * </ul> + * + * <p>Usage:</p> + * <pre>{@code + * class MyTest extends AbstractExcelTest { + * @ParameterizedTest + * @MethodSource("allFormats") + * void shouldReadAndWrite(ExcelFormat format) { + * File file = createTempFile("test", format); + * // test logic... + * } + * } + * }</pre> + */ +public abstract class AbstractExcelTest { + + @TempDir + protected Path tempDir; + + // ==================== Format Providers ==================== + + /** + * Provides all supported formats: XLSX, XLS, CSV + */ + protected static Stream<Arguments> allFormats() { + return Stream.of(Arguments.of(ExcelFormat.XLSX), Arguments.of(ExcelFormat.XLS), Arguments.of(ExcelFormat.CSV)); + } + + /** + * Provides Excel-only formats: XLSX, XLS (excludes CSV) + */ + protected static Stream<Arguments> excelFormats() { + return Stream.of(Arguments.of(ExcelFormat.XLSX), Arguments.of(ExcelFormat.XLS)); + } + + /** + * Provides formats that support styling + */ + protected static Stream<Arguments> styledFormats() { + return Stream.of(ExcelFormat.values()) + .filter(ExcelFormat::supportsStyle) + .map(Arguments::of); + } + + /** + * Provides only XLSX format (for streaming tests) + */ + protected static Stream<Arguments> xlsxOnly() { + return Stream.of(Arguments.of(ExcelFormat.XLSX)); + } + + // ==================== File Operations ==================== + + /** + * Creates a temp file with the given name and format extension. + * @param baseName base file name (without extension) + * @param format the Excel format + * @return the created file + */ + protected File createTempFile(String baseName, ExcelFormat format) { + return tempDir.resolve(baseName + format.getExtension()).toFile(); Review Comment: is that helpful to append the random value for multiple trigger in one method? ########## fesod/src/test/java/org/apache/fesod/sheet/testkit/data/RandomDataGenerator.java: ########## @@ -0,0 +1,136 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.testkit.data; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Date; +import java.util.Random; +import org.apache.fesod.sheet.model.ComplexData; +import org.apache.fesod.sheet.model.SimpleData; + +/** + * Generates random test data for fuzz testing and edge case validation. + * Helps identify issues with unexpected input values. + */ +public class RandomDataGenerator { Review Comment: As project involved RandomUtils and RandomStringUtils, which methods are not mandatory? ########## fesod/src/test/java/org/apache/fesod/sheet/model/ComplexData.java: ########## @@ -0,0 +1,105 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.model; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Date; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import org.apache.fesod.sheet.annotation.ExcelProperty; + +/** + * Complex data model for testing various data type conversions. + * Consolidated from converter tests. + */ +@Getter +@Setter +@EqualsAndHashCode +public class ComplexData { + @ExcelProperty("日期") + private Date date; + + @ExcelProperty("本地日期") + private LocalDate localDate; + + @ExcelProperty("本地日期时间") + private LocalDateTime localDateTime; + + @ExcelProperty("布尔") + private Boolean booleanData; + + @ExcelProperty("大数") + private BigDecimal bigDecimal; + + @ExcelProperty("大整数") + private BigInteger bigInteger; + + @ExcelProperty("长整型") + private long longData; + + @ExcelProperty("整型") + private Integer integerData; + + @ExcelProperty("短整型") + private Short shortData; + + @ExcelProperty("字节型") + private Byte byteData; + + @ExcelProperty("双精度浮点型") + private double doubleData; Review Comment: Why this is not the object as other fields？ ########## fesod/src/test/java/org/apache/fesod/sheet/model/ComplexData.java: ########## @@ -0,0 +1,105 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.model; + +import java.math.BigDecimal; +import java.math.BigInteger; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Date; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import org.apache.fesod.sheet.annotation.ExcelProperty; + +/** + * Complex data model for testing various data type conversions. + * Consolidated from converter tests. + */ +@Getter +@Setter +@EqualsAndHashCode +public class ComplexData { + @ExcelProperty("日期") + private Date date; + + @ExcelProperty("本地日期") + private LocalDate localDate; + + @ExcelProperty("本地日期时间") + private LocalDateTime localDateTime; + + @ExcelProperty("布尔") + private Boolean booleanData; + + @ExcelProperty("大数") + private BigDecimal bigDecimal; + + @ExcelProperty("大整数") + private BigInteger bigInteger; + + @ExcelProperty("长整型") + private long longData; + + @ExcelProperty("整型") + private Integer integerData; + + @ExcelProperty("短整型") + private Short shortData; + + @ExcelProperty("字节型") + private Byte byteData; + + @ExcelProperty("双精度浮点型") + private double doubleData; + + @ExcelProperty("浮点型") + private Float floatData; + + @ExcelProperty("字符串") + private String string; + + public ComplexData() {} + + public ComplexData(String string, Date date, Double doubleData, Boolean booleanData) { + this.string = string; + this.date = date; + this.doubleData = doubleData; + this.booleanData = booleanData; + } + + @Override + public String toString() { + return "ComplexData{" + "date=" + + date + ", localDate=" + + localDate + ", localDateTime=" + + localDateTime + ", booleanData=" Review Comment: What's the behavior of Instant ? ########## fesod/src/test/java/org/apache/fesod/sheet/testkit/base/AbstractExcelTest.java: ########## @@ -0,0 +1,153 @@ +/* + * 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. + */ + +package org.apache.fesod.sheet.testkit.base; + +import java.io.File; +import java.nio.file.Path; +import java.util.List; +import java.util.stream.Stream; +import org.apache.fesod.sheet.FesodSheet; +import org.apache.fesod.sheet.testkit.enums.ExcelFormat; +import org.apache.fesod.sheet.testkit.listeners.CollectingReadListener; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.provider.Arguments; + +/** + * Abstract base class for Excel tests providing common infrastructure. + * + * <p>Features:</p> + * <ul> + * <li>Automatic temp directory management via JUnit 5 @TempDir</li> + * <li>Format provider methods for parameterized tests</li> + * <li>Common read/write operations</li> + * <li>Format-aware file creation</li> + * </ul> + * + * <p>Usage:</p> + * <pre>{@code + * class MyTest extends AbstractExcelTest { + * @ParameterizedTest + * @MethodSource("allFormats") + * void shouldReadAndWrite(ExcelFormat format) { + * File file = createTempFile("test", format); + * // test logic... + * } + * } + * }</pre> + */ +public abstract class AbstractExcelTest { + + @TempDir + protected Path tempDir; + + // ==================== Format Providers ==================== + + /** + * Provides all supported formats: XLSX, XLS, CSV + */ + protected static Stream<Arguments> allFormats() { + return Stream.of(Arguments.of(ExcelFormat.XLSX), Arguments.of(ExcelFormat.XLS), Arguments.of(ExcelFormat.CSV)); + } + + /** + * Provides Excel-only formats: XLSX, XLS (excludes CSV) + */ + protected static Stream<Arguments> excelFormats() { + return Stream.of(Arguments.of(ExcelFormat.XLSX), Arguments.of(ExcelFormat.XLS)); + } + + /** + * Provides formats that support styling + */ + protected static Stream<Arguments> styledFormats() { + return Stream.of(ExcelFormat.values()) + .filter(ExcelFormat::supportsStyle) + .map(Arguments::of); + } + + /** + * Provides only XLSX format (for streaming tests) + */ + protected static Stream<Arguments> xlsxOnly() { + return Stream.of(Arguments.of(ExcelFormat.XLSX)); + } + + // ==================== File Operations ==================== + + /** + * Creates a temp file with the given name and format extension. + * @param baseName base file name (without extension) + * @param format the Excel format + * @return the created file + */ + protected File createTempFile(String baseName, ExcelFormat format) { + return tempDir.resolve(baseName + format.getExtension()).toFile(); + } + + /** + * Creates a temp file with unique name based on test method. + */ + protected File createTempFile(ExcelFormat format, org.junit.jupiter.api.TestInfo testInfo) { + String methodName = testInfo.getTestMethod().map(m -> m.getName()).orElse("test"); + return createTempFile(methodName, format); + } + + // ==================== Common Operations ==================== + + /** + * Writes data to file using FesodSheet. + */ + protected <T> void writeData(File file, Class<T> clazz, List<T> data) { Review Comment: will we have the unit test for such test method? otherwise why protected ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
