-
Notifications
You must be signed in to change notification settings - Fork 508
fix: fix error in using relativeHeadRowIndex for writing tables only #810
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.
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
Open
32154678925
wants to merge
11
commits into
apache:main
Choose a base branch
from
32154678925:fix/fix-relativeHeadRowIndex-invalid-#794
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+254
−3
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
bce94fd
fix: fix error in using relativeHeadRowIndex for writing tables only(…
32154678925 d4c9711
fix: fix error in using relativeHeadRowIndex for writing tables only(…
32154678925 b557200
Merge branch 'main' into fix/fix-relativeHeadRowIndex-invalid-#794
alaahong a3bf3a2
Merge branch 'main' into fix/fix-relativeHeadRowIndex-invalid-#794
alaahong e1853e8
Merge branch 'main' into fix/fix-relativeHeadRowIndex-invalid-#794
bb22044
Modify style issues and add unit tests
32154678925 9e4b31f
test: enhance WriteTableTest with parameterized assertions for XLS/XLSX
32154678925 c262f78
test code format
32154678925 96b677d
Merge branch 'main' into fix/fix-relativeHeadRowIndex-invalid-#794
32154678925 319d2a7
Merge branch 'main' into fix/fix-relativeHeadRowIndex-invalid-#794
bengbengbalabalabeng 65bef47
Merge branch 'main' into fix/fix-relativeHeadRowIndex-invalid-#794
32154678925 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
247 changes: 247 additions & 0 deletions
247
fesod-sheet/src/test/java/org/apache/fesod/sheet/writesheet/WriteTableTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,247 @@ | ||
| package org.apache.fesod.sheet.writesheet; | ||
|
|
||
| import java.io.File; | ||
| import java.lang.reflect.Field; | ||
| import java.util.*; | ||
| import org.apache.fesod.sheet.ExcelWriter; | ||
| import org.apache.fesod.sheet.FesodSheet; | ||
| import org.apache.fesod.sheet.annotation.ExcelProperty; | ||
| import org.apache.fesod.sheet.support.ExcelTypeEnum; | ||
| import org.apache.fesod.sheet.util.TestFileUtil; | ||
| import org.apache.fesod.sheet.write.builder.ExcelWriterBuilder; | ||
| import org.apache.fesod.sheet.write.builder.ExcelWriterSheetBuilder; | ||
| import org.apache.fesod.sheet.write.metadata.WriteSheet; | ||
| import org.apache.fesod.sheet.write.metadata.WriteTable; | ||
| import org.apache.poi.ss.usermodel.*; | ||
| import org.junit.jupiter.api.Assertions; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.EnumSource; | ||
|
|
||
| public class WriteTableTest { | ||
|
|
||
| private static final String HEADER_STRING; | ||
|
|
||
| static { | ||
| try { | ||
| Field field = WriteSheetData.class.getDeclaredField("string"); | ||
| ExcelProperty annotation = field.getAnnotation(ExcelProperty.class); | ||
| HEADER_STRING = annotation.value()[0]; | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("Failed to resolve @ExcelProperty header from WriteSheetData.string", e); | ||
| } | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @EnumSource( | ||
| value = ExcelTypeEnum.class, | ||
| names = {"XLS", "XLSX"}) | ||
| public void testWriteTableWithTitle(ExcelTypeEnum excelType) throws Exception { | ||
| Random random = new Random(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Random numbers may lead to non-deterministic, it is recommended to use @MethodSource to provide N sets of representative boundary values for testing. |
||
| int offsetA = random.nextInt(10); | ||
| int offsetB = random.nextInt(10); | ||
| int sizeA = random.nextInt(10) + 1; | ||
| int sizeB = random.nextInt(10) + 1; | ||
|
|
||
| File testFile = TestFileUtil.createNewFile("writesheet/write-table-title" + excelType.getValue()); | ||
|
|
||
| // ── write ──────────────────────────────────────────────────────────── | ||
|
|
||
| ExcelWriterBuilder write = FesodSheet.write(testFile); | ||
| ExcelWriterSheetBuilder sheetBuilder = write.sheet(0, "0"); | ||
| WriteSheet sheet = sheetBuilder.build(); | ||
|
|
||
| WriteTable tableA = sheetBuilder | ||
| .table(0) | ||
| .head(WriteSheetData.class) | ||
| .relativeHeadRowIndex(offsetA) | ||
| .build(); | ||
| WriteTable tableB = sheetBuilder | ||
| .table(1) | ||
| .relativeHeadRowIndex(offsetB) | ||
| .head(WriteSheetData.class) | ||
| .build(); | ||
|
|
||
| try (ExcelWriter writer = write.build()) { | ||
| writer.write(getList(sizeA, "A-"), sheet, tableA); | ||
| writer.write(getList(sizeB, "B-"), sheet, tableB); | ||
| writer.finish(); | ||
| } | ||
|
|
||
| Assertions.assertTrue(testFile.exists(), "Written file should exist"); | ||
| Assertions.assertTrue(testFile.length() > 0, "Written file should not be empty"); | ||
|
|
||
| // ── read & assert ──────────────────────────────────────────────────── | ||
|
|
||
| try (Workbook workbook = WorkbookFactory.create(testFile)) { | ||
| Sheet excelSheet = workbook.getSheetAt(0); | ||
| Assertions.assertNotNull(excelSheet, "Sheet '0' should exist"); | ||
|
|
||
| Map<Integer, String> expected = buildExpectedWithTitle(offsetA, offsetB, sizeA, sizeB); | ||
| int totalRows = expected.size(); | ||
| Assertions.assertEquals( | ||
| totalRows, | ||
| excelSheet.getPhysicalNumberOfRows(), | ||
| "Sheet should have exactly " + totalRows + " rows " | ||
| + "(offsets: A=" + offsetA + " B=" + offsetB | ||
| + ", sizes: A=" + sizeA + " B=" + sizeB + ")"); | ||
|
|
||
| for (Map.Entry<Integer, String> entry : expected.entrySet()) { | ||
| int rowIdx = entry.getKey(); | ||
| String expectedValue = entry.getValue(); | ||
| Row row = excelSheet.getRow(rowIdx); | ||
| Assertions.assertNotNull( | ||
| row, | ||
| "Expected row " + rowIdx + " with value '" + expectedValue | ||
| + "' (A:" + offsetA + "/" + sizeA | ||
| + " B:" + offsetB + "/" + sizeB + ")"); | ||
| Cell cell = row.getCell(0); | ||
| Assertions.assertNotNull(cell, "Expected cell at row " + rowIdx); | ||
| Assertions.assertEquals( | ||
| expectedValue, | ||
| cell.getStringCellValue(), | ||
| "Row " + rowIdx + " expected '" + expectedValue + "' " | ||
| + "(A:" + offsetA + "/" + sizeA | ||
| + " B:" + offsetB + "/" + sizeB + ")"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @EnumSource( | ||
| value = ExcelTypeEnum.class, | ||
| names = {"XLS", "XLSX"}) | ||
| public void testWriteTableWithoutTitle(ExcelTypeEnum excelType) throws Exception { | ||
| Random random = new Random(); | ||
| int offsetC = random.nextInt(10); | ||
| int offsetA = random.nextInt(10); | ||
| int offsetB = random.nextInt(10); | ||
| int sizeC = random.nextInt(10) + 1; | ||
| int sizeA = random.nextInt(10) + 1; | ||
| int sizeB = random.nextInt(10) + 1; | ||
|
|
||
| File testFile = TestFileUtil.createNewFile("writesheet/write-table-notitle" + excelType.getValue()); | ||
|
|
||
| // ── write ──────────────────────────────────────────────────────────── | ||
|
|
||
| ExcelWriterBuilder write = FesodSheet.write(testFile); | ||
| ExcelWriterSheetBuilder sheetBuilder = write.sheet(0, "0"); | ||
| WriteSheet sheet = sheetBuilder.build(); | ||
|
|
||
| WriteSheet collect = sheetBuilder.relativeHeadRowIndex(offsetC).build(); | ||
| WriteTable tableA = sheetBuilder.table(0).relativeHeadRowIndex(offsetA).build(); | ||
| WriteTable tableB = sheetBuilder.table(1).relativeHeadRowIndex(offsetB).build(); | ||
|
|
||
| try (ExcelWriter writer = write.build()) { | ||
| writer.write(getList(sizeC, "C-"), collect); | ||
| writer.write(getList(sizeA, "A-"), sheet, tableA); | ||
| writer.write(getList(sizeB, "B-"), sheet, tableB); | ||
| writer.finish(); | ||
| } | ||
|
|
||
| Assertions.assertTrue(testFile.exists(), "Written file should exist"); | ||
| Assertions.assertTrue(testFile.length() > 0, "Written file should not be empty"); | ||
|
|
||
| // ── read & assert ──────────────────────────────────────────────────── | ||
|
|
||
| try (Workbook workbook = WorkbookFactory.create(testFile)) { | ||
| Sheet excelSheet = workbook.getSheetAt(0); | ||
| Assertions.assertNotNull(excelSheet, "Sheet '0' should exist"); | ||
|
|
||
| Map<Integer, String> expected = buildExpectedWithoutTitle(offsetC, offsetA, offsetB, sizeC, sizeA, sizeB); | ||
| int totalRows = expected.size(); | ||
| Assertions.assertEquals( | ||
| totalRows, | ||
| excelSheet.getPhysicalNumberOfRows(), | ||
| "Sheet should have exactly " + totalRows + " rows " | ||
| + "(C:" + offsetC + "/" + sizeC | ||
| + " A:" + offsetA + "/" + sizeA | ||
| + " B:" + offsetB + "/" + sizeB + ")"); | ||
|
|
||
| for (Map.Entry<Integer, String> entry : expected.entrySet()) { | ||
| int rowIdx = entry.getKey(); | ||
| String expectedValue = entry.getValue(); | ||
| Row row = excelSheet.getRow(rowIdx); | ||
| Assertions.assertNotNull( | ||
| row, | ||
| "Expected row " + rowIdx + " with value '" + expectedValue | ||
| + "' (C:" + offsetC + "/" + sizeC | ||
| + " A:" + offsetA + "/" + sizeA | ||
| + " B:" + offsetB + "/" + sizeB + ")"); | ||
| Cell cell = row.getCell(0); | ||
| Assertions.assertNotNull(cell, "Expected cell at row " + rowIdx); | ||
| Assertions.assertEquals( | ||
| expectedValue, | ||
| cell.getStringCellValue(), | ||
| "Row " + rowIdx + " expected '" + expectedValue + "' " | ||
| + "(C:" + offsetC + "/" + sizeC | ||
| + " A:" + offsetA + "/" + sizeA | ||
| + " B:" + offsetB + "/" + sizeB + ")"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ── expected-row builders ──────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * Build expected row map for with-title tables. | ||
| * Tables are sequential: B starts after A's last row + offsetB empty rows. | ||
| */ | ||
| private static Map<Integer, String> buildExpectedWithTitle(int offsetA, int offsetB, int sizeA, int sizeB) { | ||
| Map<Integer, String> expected = new LinkedHashMap<>(); | ||
|
|
||
| // Table A: head at offsetA, data at offsetA+1 ... offsetA+sizeA | ||
| expected.put(offsetA, HEADER_STRING); | ||
| for (int i = 0; i < sizeA; i++) { | ||
| expected.put(offsetA + 1 + i, "A-" + i); | ||
| } | ||
|
|
||
| // Table B: offsetB empty rows after A's last row, then head + data | ||
| int bHeadRow = offsetA + sizeA + offsetB + 1; | ||
| expected.put(bHeadRow, HEADER_STRING); | ||
| for (int i = 0; i < sizeB; i++) { | ||
| expected.put(bHeadRow + 1 + i, "B-" + i); | ||
| } | ||
|
|
||
| return expected; | ||
| } | ||
|
|
||
| /** | ||
| * Build expected row map for headerless writes. | ||
| * Tables are sequential: each starts after the previous one's last row + offset. | ||
| */ | ||
| private static Map<Integer, String> buildExpectedWithoutTitle( | ||
| int offsetC, int offsetA, int offsetB, int sizeC, int sizeA, int sizeB) { | ||
| Map<Integer, String> expected = new LinkedHashMap<>(); | ||
|
|
||
| // collect: data at offsetC ... offsetC+sizeC-1 | ||
| for (int i = 0; i < sizeC; i++) { | ||
| expected.put(offsetC + i, "C-" + i); | ||
| } | ||
|
|
||
| // tableA: starts after collect's last row + offsetA empty rows | ||
| int aStartRow = offsetC + sizeC + offsetA; | ||
| for (int i = 0; i < sizeA; i++) { | ||
| expected.put(aStartRow + i, "A-" + i); | ||
| } | ||
|
|
||
| // tableB: starts after A's last row + offsetB empty rows | ||
| int bStartRow = aStartRow + sizeA + offsetB; | ||
| for (int i = 0; i < sizeB; i++) { | ||
| expected.put(bStartRow + i, "B-" + i); | ||
| } | ||
|
|
||
| return expected; | ||
| } | ||
|
|
||
| // ── helper ─────────────────────────────────────────────────────────────── | ||
|
|
||
| private static List<WriteSheetData> getList(int size, String prefix) { | ||
| List<WriteSheetData> dataList = new ArrayList<>(); | ||
| for (int j = 0; j < size; j++) { | ||
| WriteSheetData data = new WriteSheetData(); | ||
| data.setString(prefix + j); | ||
| dataList.add(data); | ||
| } | ||
| return dataList; | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please supplement the copyright header. license-header.txt