From 3708d21b743fc49abffd6bbf3e3163f21ff2d094 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 10 Jan 2025 14:06:58 -0600 Subject: [PATCH 1/3] fix: Chart subscriptions shouldn't reverse positions Fixes DH-18389 --- .../java/io/deephaven/web/client/api/widget/plot/ChartData.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/ChartData.java b/web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/ChartData.java index bd08d045bf2..e0cb3393bc6 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/ChartData.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/widget/plot/ChartData.java @@ -72,7 +72,7 @@ public void update(AbstractTableSubscription.SubscriptionEventData tableData) { private void replaceDataRange(SubscriptionTableData tableData, Range positions) { RangeSet keys = tableData.getFullIndex().getRange() - .subsetForPositions(RangeSet.ofRange(positions.getFirst(), positions.getLast()), true); + .subsetForPositions(RangeSet.ofRange(positions.getFirst(), positions.getLast()), false); // we don't touch the indexes at all, only need to walk each column and replace values in this range for (Entry, JsArray>> columnMap : cachedData.entrySet()) { Column col = table.findColumn(columnMap.getKey()); From 9239b40a0ebb934bc11ae08f26d517615955d301 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 10 Jan 2025 16:48:57 -0600 Subject: [PATCH 2/3] test: Simple ChartData tests for JS API Follow-up #6546 --- .../web/ClientIntegrationTestSuite.java | 2 + .../deephaven/web/CustomTransportTest.gwt.xml | 4 + .../client/api/grpc/GrpcTransportTestGwt.java | 3 +- .../api/widget/plot/ChartDataTestGwt.java | 87 +++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 web/client-api/src/test/java/io/deephaven/web/CustomTransportTest.gwt.xml create mode 100644 web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java diff --git a/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java b/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java index 3c3aa8791ca..7f3eb1c60a4 100644 --- a/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java +++ b/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java @@ -9,6 +9,7 @@ import io.deephaven.web.client.api.storage.JsStorageServiceTestGwt; import io.deephaven.web.client.api.subscription.ConcurrentTableTestGwt; import io.deephaven.web.client.api.subscription.ViewportTestGwt; +import io.deephaven.web.client.api.widget.plot.ChartDataTestGwt; import io.deephaven.web.client.fu.LazyPromiseTestGwt; import junit.framework.Test; import junit.framework.TestSuite; @@ -32,6 +33,7 @@ public static Test suite() { suite.addTestSuite(InputTableTestGwt.class); suite.addTestSuite(ColumnStatisticsTestGwt.class); suite.addTestSuite(GrpcTransportTestGwt.class); + suite.addTestSuite(ChartDataTestGwt.class); // This should be a unit test, but it requires a browser environment to run on GWT 2.9 // GWT 2.9 doesn't have proper bindings for Promises in HtmlUnit, so we need to use the IntegrationTest suite diff --git a/web/client-api/src/test/java/io/deephaven/web/CustomTransportTest.gwt.xml b/web/client-api/src/test/java/io/deephaven/web/CustomTransportTest.gwt.xml new file mode 100644 index 00000000000..36e00484fd6 --- /dev/null +++ b/web/client-api/src/test/java/io/deephaven/web/CustomTransportTest.gwt.xml @@ -0,0 +1,4 @@ + + + + diff --git a/web/client-api/src/test/java/io/deephaven/web/client/api/grpc/GrpcTransportTestGwt.java b/web/client-api/src/test/java/io/deephaven/web/client/api/grpc/GrpcTransportTestGwt.java index 020c65b771a..2a9ff5d5792 100644 --- a/web/client-api/src/test/java/io/deephaven/web/client/api/grpc/GrpcTransportTestGwt.java +++ b/web/client-api/src/test/java/io/deephaven/web/client/api/grpc/GrpcTransportTestGwt.java @@ -16,7 +16,8 @@ public class GrpcTransportTestGwt extends AbstractAsyncGwtTestCase { @Override public String getModuleName() { - return "io.deephaven.web.DeephavenIntegrationTest"; + // This test runs in its own module to avoid risking poisoning other tests with its broken custom transports + return "io.deephaven.web.CustomTransportTest"; } /** diff --git a/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java b/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java new file mode 100644 index 00000000000..c2d0d568822 --- /dev/null +++ b/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java @@ -0,0 +1,87 @@ +package io.deephaven.web.client.api.widget.plot; + +import elemental2.promise.Promise; +import io.deephaven.web.client.api.AbstractAsyncGwtTestCase; +import io.deephaven.web.client.api.subscription.AbstractTableSubscription; +import io.deephaven.web.client.api.subscription.TableSubscription; + +public class ChartDataTestGwt extends AbstractAsyncGwtTestCase { + private final AbstractAsyncGwtTestCase.TableSourceBuilder tables = new AbstractAsyncGwtTestCase.TableSourceBuilder() + .script("from deephaven import time_table") + .script("appending_table", "time_table('PT0.1s').update([\"A = i % 2\", \"B = `` + A\"])") + .script("updating_table", "appending_table.last_by('A')"); + + @Override + public String getModuleName() { + return "io.deephaven.web.DeephavenIntegrationTest"; + } + + public void testAppendingChartData() { + connect(tables) + .then(table("appending_table")) + .then(table -> { + ChartData chartData = new ChartData(table); + TableSubscription sub = table.subscribe(table.getColumns()); + + // Handle at least one event with data + int[] count = {0}; + + return new Promise((resolve, reject) -> { + delayTestFinish(12301); + sub.addEventListener(TableSubscription.EVENT_UPDATED, event -> { + AbstractTableSubscription.SubscriptionEventData data = (AbstractTableSubscription.SubscriptionEventData) event.getDetail(); + chartData.update(data); + + if (count[0] > 0) { + // Don't test on the first update, could still be empty + assertTrue(chartData.getColumn("A", arr -> arr, data).length > 0); + + resolve.onInvoke(data); + } + count[0]++; + }); + }).then(data -> { + sub.close(); + table.close(); + return Promise.resolve(data); + }); + }) + .then(this::finish).catch_(this::report); + } + + public void testModifiedChartData() { + connect(tables) + .then(table("updating_table")) + .then(table -> { + ChartData chartData = new ChartData(table); + TableSubscription sub = table.subscribe(table.getColumns()); + + int[] count = {0}; + return new Promise((resolve, reject) -> { + delayTestFinish(12302); + sub.addEventListener(TableSubscription.EVENT_UPDATED, event -> { + + AbstractTableSubscription.SubscriptionEventData data = (AbstractTableSubscription.SubscriptionEventData) event.getDetail(); + + chartData.update(data); + if (count[0] > 0) { + // Don't test on the first update, could still be empty + assertTrue(chartData.getColumn("A", arr -> arr, data).length > 0); + } + + if (count[0] > 2) { + // We've seen at least three updates, and must have modified rows at least once + resolve.onInvoke((AbstractTableSubscription.SubscriptionEventData) event.getDetail()); + } + count[0]++; + }); + }) + .then(data -> { + sub.close(); + table.close(); + return Promise.resolve(data); + }); + }) + .then(this::finish).catch_(this::report); + } +} From 1321537d19f8715f4da8865431d9488b790ff58e Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 10 Jan 2025 19:44:20 -0600 Subject: [PATCH 3/3] spotless --- .../web/client/api/widget/plot/ChartDataTestGwt.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java b/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java index c2d0d568822..d9faf49257e 100644 --- a/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java +++ b/web/client-api/src/test/java/io/deephaven/web/client/api/widget/plot/ChartDataTestGwt.java @@ -1,3 +1,6 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// package io.deephaven.web.client.api.widget.plot; import elemental2.promise.Promise; @@ -29,7 +32,8 @@ public void testAppendingChartData() { return new Promise((resolve, reject) -> { delayTestFinish(12301); sub.addEventListener(TableSubscription.EVENT_UPDATED, event -> { - AbstractTableSubscription.SubscriptionEventData data = (AbstractTableSubscription.SubscriptionEventData) event.getDetail(); + AbstractTableSubscription.SubscriptionEventData data = + (AbstractTableSubscription.SubscriptionEventData) event.getDetail(); chartData.update(data); if (count[0] > 0) { @@ -61,7 +65,8 @@ public void testModifiedChartData() { delayTestFinish(12302); sub.addEventListener(TableSubscription.EVENT_UPDATED, event -> { - AbstractTableSubscription.SubscriptionEventData data = (AbstractTableSubscription.SubscriptionEventData) event.getDetail(); + AbstractTableSubscription.SubscriptionEventData data = + (AbstractTableSubscription.SubscriptionEventData) event.getDetail(); chartData.update(data); if (count[0] > 0) {