codeant-ai-for-open-source[bot] commented on code in PR #38080:
URL: https://github.com/apache/superset/pull/38080#discussion_r2977526785
##########
superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx:
##########
@@ -695,6 +696,187 @@ test('values ββfrom the 3rd level of the hierarchy with a
subtotal at the to
});
});
+type TestData = {
+ [key: string]: number | string | null;
+};
+
+const createMockAggregator =
+ (data: TestData) =>
+ (key: string[], _context: never[]): unknown => {
+ const keyStr = key.join('|');
+ return data[keyStr] ?? null;
+ };
+
+test('should sort flat keys in ascending order', () => {
+ const keys: string[][] = [['A'], ['C'], ['B']];
+ const data = {
+ A: 30,
+ B: 10,
+ C: 20,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, true);
+
+ expect(keys).toEqual([['B'], ['C'], ['A']]);
+});
+
+test('should sort flat keys in descending order', () => {
+ const keys: string[][] = [['A'], ['C'], ['B']];
+ const data = {
+ A: 30,
+ B: 10,
+ C: 20,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, false);
+
+ expect(keys).toEqual([['A'], ['C'], ['B']]);
+});
+
+test('should place subtotal at top when top=true and ascending', () => {
+ const keys: string[][] = [
+ ['Region', 'City1'],
+ ['Region'],
+ ['Region', 'City2'],
+ ];
+ const data = {
+ 'Region|City1': 100,
+ 'Region|City2': 50,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), true, true);
+
+ expect(keys[0]).toEqual(['Region']);
+ expect(keys[1]).toEqual(['Region', 'City2']);
+ expect(keys[2]).toEqual(['Region', 'City1']);
+});
+
+test('should place subtotal at bottom when top=false and descending', () => {
+ const keys: string[][] = [
+ ['Region', 'City1'],
+ ['Region'],
+ ['Region', 'City2'],
+ ];
+ const data = {
+ 'Region|City1': 100,
+ 'Region|City2': 50,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, false);
+
+ expect(keys[0]).toEqual(['Region', 'City1']);
+ expect(keys[1]).toEqual(['Region', 'City2']);
+ expect(keys[2]).toEqual(['Region']);
+});
+
+test('should use alphabetical order for terminals with equal values', () => {
+ const keys: string[][] = [
+ ['Group', 'Apple'],
+ ['Group', 'Banana'],
+ ['Group', 'Cherry'],
+ ];
+ const data = {
+ 'Group|Apple': 50,
+ 'Group|Banana': 50,
+ 'Group|Cherry': 50,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, true);
+
+ expect(keys).toEqual([
+ ['Group', 'Apple'],
+ ['Group', 'Banana'],
+ ['Group', 'Cherry'],
+ ]);
+});
+
+test('should handle null values gracefully', () => {
+ const keys: string[][] = [['A'], ['B'], ['C']];
+ const data = {
+ A: 100,
+ B: null,
+ C: 50,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, true);
+ expect(keys).toEqual([['B'], ['C'], ['A']]);
+});
+
+test('should handle string numbers', () => {
+ const keys: string[][] = [['A'], ['B'], ['C']];
+ const data = {
+ A: '100',
+ B: '50',
+ C: '200',
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, false);
+ expect(keys).toEqual([['C'], ['A'], ['B']]);
+});
+
+test('should handle NaN values', () => {
+ const keys: string[][] = [['A'], ['B'], ['C']];
+ const data = {
+ A: 100,
+ B: NaN,
+ C: 50,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), false, true);
+ expect(keys).toEqual([['B'], ['C'], ['A']]);
+});
+
+test('should handle single key', () => {
+ const keys: string[][] = [['OnlyKey']];
+ const data = { OnlyKey: 42 };
+
+ groupingValueSort(keys, createMockAggregator(data), false, true);
+ expect(keys).toEqual([['OnlyKey']]);
+});
+
+test('should handle empty keys array', () => {
+ const keys: string[][] = [];
+ const data = {};
+
+ groupingValueSort(keys, createMockAggregator(data), false, true);
+ expect(keys).toEqual([]);
+});
+
+test('should handle product categories with subcategories', () => {
+ const keys: string[][] = [
+ ['Electronics'],
+ ['Electronics', 'Phones'],
+ ['Electronics', 'Phones', 'iPhone'],
+ ['Electronics', 'Phones', 'Samsung'],
+ ['Electronics', 'Laptops'],
+ ['Electronics', 'Laptops', 'MacBook'],
+ ['Clothing'],
+ ['Clothing', 'Shirts'],
+ ['Clothing', 'Shirts', 'T-Shirt'],
+ ['Clothing', 'Pants'],
+ ['Clothing', 'Pants', 'Jeans'],
+ ];
+ const data = {
+ 'Electronics|Phones|iPhone': 500,
+ 'Electronics|Phones|Samsung': 400,
+ 'Electronics|Laptops|MacBook': 1200,
+ 'Clothing|Shirts|T-Shirt': 1400,
+ 'Clothing|Pants|Jeans': 1150,
+ };
+
+ groupingValueSort(keys, createMockAggregator(data), true, true);
+
+ expect(keys[0]).toEqual(['Electronics']);
+ expect(keys[1]).toEqual(['Electronics', 'Phones']);
+ expect(keys[2]).toEqual(['Electronics', 'Phones', 'Samsung']);
+ expect(keys[3]).toEqual(['Electronics', 'Phones', 'iPhone']);
+ expect(keys[4]).toEqual(['Electronics', 'Laptops']);
+ expect(keys[5]).toEqual(['Electronics', 'Laptops', 'MacBook']);
+ expect(keys[6]).toEqual(['Clothing']);
+ expect(keys[7]).toEqual(['Clothing', 'Pants']);
+ expect(keys[8]).toEqual(['Clothing', 'Pants', 'Jeans']);
+ expect(keys[9]).toEqual(['Clothing', 'Shirts']);
+ expect(keys[10]).toEqual(['Clothing', 'Shirts', 'T-Shirt']);
test('getCellColor derives readable text from the winning background', () => {
Review Comment:
**Suggestion:** The product-categories test is not closed before the next
`test(...)` declaration, which makes the next test execute inside another test
body. In Jest this causes nested test registration errors at runtime and
prevents the suite from running correctly. Close the first test block before
starting the next one. [logic error]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Jest run fails for pivot-table test file.
- β οΈ Frontend CI test stage can be blocked.
- β οΈ Subsequent tests may not register correctly.
```
</details>
```suggestion
});
```
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Run frontend tests using `npm test -- tableRenders.test.tsx` (test script
defined at
`superset-frontend/package.json:85`).
2. Jest picks this file through `testRegex` in
`superset-frontend/jest.config.js:22-23`,
so `tableRenders.test.tsx` is executed normally.
3. In
`superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx:845`,
test `should handle product categories with subcategories` opens a callback.
4. At `.../tableRenders.test.tsx:880`, a new `test('getCellColor...')` is
declared before
closing the prior callback (missing `});` after line 879).
5. Jest registers a test inside another test callback and fails runtime
registration
(nested test error), so this test file cannot run correctly.
```
</details>
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx
**Line:** 880:880
**Comment:**
*Logic Error: The product-categories test is not closed before the next
`test(...)` declaration, which makes the next test execute inside another test
body. In Jest this causes nested test registration errors at runtime and
prevents the suite from running correctly. Close the first test block before
starting the next one.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38080&comment_hash=4b7e0e67ffe95bc8429b4178f48c76dafe0e24abd9ddabe6d363f9351b87db7b&reaction=like'>π</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38080&comment_hash=4b7e0e67ffe95bc8429b4178f48c76dafe0e24abd9ddabe6d363f9351b87db7b&reaction=dislike'>π</a>
--
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]
