alexandrusoare commented on code in PR #37532:
URL: https://github.com/apache/superset/pull/37532#discussion_r2745526858
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/controlPanel.test.ts:
##########
@@ -17,188 +17,204 @@
* under the License.
*/
import controlPanel from '../../../src/Timeseries/Regular/Bar/controlPanel';
-
-describe('Bar Chart Control Panel', () => {
- describe('x_axis_time_format control', () => {
- it('should include x_axis_time_format control in the panel', () => {
- const config = controlPanel;
-
- // Look for x_axis_time_format control in all sections and rows
- let foundTimeFormatControl = false;
-
- for (const section of config.controlPanelSections) {
- if (section && section.controlSetRows) {
- for (const row of section.controlSetRows) {
- for (const control of row) {
- if (
- typeof control === 'object' &&
- control !== null &&
- 'name' in control &&
- control.name === 'x_axis_time_format'
- ) {
- foundTimeFormatControl = true;
- break;
- }
- }
- if (foundTimeFormatControl) break;
- }
- if (foundTimeFormatControl) break;
- }
- }
-
- expect(foundTimeFormatControl).toBe(true);
- });
-
- it('should have correct default value for x_axis_time_format', () => {
- const config = controlPanel;
-
- // Find the x_axis_time_format control
- let timeFormatControl: any = null;
-
- for (const section of config.controlPanelSections) {
- if (section && section.controlSetRows) {
- for (const row of section.controlSetRows) {
- for (const control of row) {
- if (
- typeof control === 'object' &&
- control !== null &&
- 'name' in control &&
- control.name === 'x_axis_time_format'
- ) {
- timeFormatControl = control;
- break;
- }
- }
- if (timeFormatControl) break;
+import {
+ BarChartStackControlOptions,
+ StackControlsValue,
+} from '../../../src/constants';
+
+const config = controlPanel;
+
+const getControl = (controlName: string) => {
+ for (const section of config.controlPanelSections) {
+ if (section && section.controlSetRows) {
+ for (const row of section.controlSetRows) {
+ for (const control of row) {
+ if (
+ typeof control === 'object' &&
+ control !== null &&
+ 'name' in control &&
+ control.name === controlName
+ ) {
+ return control;
}
- if (timeFormatControl) break;
}
}
+ }
+ }
+
+ return null;
+};
+
+// Mock getStandardizedControls
+jest.mock('@superset-ui/chart-controls', () => {
+ const actual = jest.requireActual('@superset-ui/chart-controls');
+ return {
+ ...actual,
+ getStandardizedControls: jest.fn(() => ({
+ popAllMetrics: jest.fn(() => []),
+ popAllColumns: jest.fn(() => []),
+ })),
+ };
+});
- expect(timeFormatControl).toBeDefined();
- expect(timeFormatControl.config).toBeDefined();
- expect(timeFormatControl.config.default).toBe('smart_date');
- });
-
- it('should have visibility function for x_axis_time_format', () => {
- const config = controlPanel;
-
- // Find the x_axis_time_format control
- let timeFormatControl: any = null;
-
- for (const section of config.controlPanelSections) {
- if (section && section.controlSetRows) {
- for (const row of section.controlSetRows) {
- for (const control of row) {
- if (
- typeof control === 'object' &&
- control !== null &&
- 'name' in control &&
- control.name === 'x_axis_time_format'
- ) {
- timeFormatControl = control;
- break;
- }
- }
- if (timeFormatControl) break;
- }
- if (timeFormatControl) break;
- }
- }
+test('should include x_axis_time_format control in the panel', () => {
+ const timeFormatControl = getControl('x_axis_time_format');
+ expect(timeFormatControl).toBeDefined();
+});
- expect(timeFormatControl).toBeDefined();
- expect(timeFormatControl.config.visibility).toBeDefined();
- expect(typeof timeFormatControl.config.visibility).toBe('function');
-
- // The visibility function exists - the exact logic is tested implicitly
through UI behavior
- // The important part is that the control has proper visibility
configuration
- });
-
- it('should have proper control configuration', () => {
- const config = controlPanel;
-
- // Find the x_axis_time_format control
- let timeFormatControl: any = null;
-
- for (const section of config.controlPanelSections) {
- if (section && section.controlSetRows) {
- for (const row of section.controlSetRows) {
- for (const control of row) {
- if (
- typeof control === 'object' &&
- control !== null &&
- 'name' in control &&
- control.name === 'x_axis_time_format'
- ) {
- timeFormatControl = control;
- break;
- }
- }
- if (timeFormatControl) break;
- }
- if (timeFormatControl) break;
- }
- }
+test('should have correct default value for x_axis_time_format', () => {
+ const timeFormatControl: any = getControl('x_axis_time_format');
+ expect(timeFormatControl).toBeDefined();
+ expect(timeFormatControl.config).toBeDefined();
+ expect(timeFormatControl.config.default).toBe('smart_date');
+});
- expect(timeFormatControl).toBeDefined();
- expect(timeFormatControl.config).toMatchObject({
- default: 'smart_date',
- disableStash: true,
- resetOnHide: false,
- });
+test('should have visibility function for x_axis_time_format', () => {
+ const timeFormatControl: any = getControl('x_axis_time_format');
+ expect(timeFormatControl).toBeDefined();
+ expect(timeFormatControl.config.visibility).toBeDefined();
+ expect(typeof timeFormatControl.config.visibility).toBe('function');
+});
- // Should have a description that includes D3 time format docs
- expect(timeFormatControl.config.description).toContain('D3');
- });
+test('should have proper control configuration for x_axis_time_format', () => {
Review Comment:
What do you think about combining some of the tests? For example for
x_axis_time_format, we can check all the properties in one test since we are
not interacting with it rather than having 3-4 separate tests
--
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]
