From 9f39efa231555c215d3336aa09f20753bd242efa Mon Sep 17 00:00:00 2001
From: naisila <nicypp@gmail.com>
Date: Tue, 14 Nov 2023 12:30:05 +0300
Subject: [PATCH] Address reviews

---
 src/test/regress/citus_tests/run_test.py      |  2 +-
 .../remove_cluster_management_nodes.out       | 37 -------------------
 .../expected/remove_non_default_nodes.out     | 13 +++++++
 src/test/regress/split_schedule               |  2 +-
 .../sql/remove_cluster_management_nodes.sql   |  7 ----
 .../regress/sql/remove_non_default_nodes.sql  |  8 ++++
 6 files changed, 23 insertions(+), 46 deletions(-)
 delete mode 100644 src/test/regress/expected/remove_cluster_management_nodes.out
 create mode 100644 src/test/regress/expected/remove_non_default_nodes.out
 delete mode 100644 src/test/regress/sql/remove_cluster_management_nodes.sql
 create mode 100644 src/test/regress/sql/remove_non_default_nodes.sql

diff --git a/src/test/regress/citus_tests/run_test.py b/src/test/regress/citus_tests/run_test.py
index 979098221..be3529c19 100755
--- a/src/test/regress/citus_tests/run_test.py
+++ b/src/test/regress/citus_tests/run_test.py
@@ -184,7 +184,7 @@ DEPS = {
     "multi_tenant_isolation_nonblocking": TestDeps(
         "minimal_schedule", ["multi_data_types", "remove_coordinator_from_metadata"]
     ),
-    "remove_cluster_management_nodes": TestDeps(
+    "remove_non_default_nodes": TestDeps(
         None, ["multi_mx_node_metadata", "multi_cluster_management"], repeatable=False
     ),
     "citus_split_shard_columnar_partitioned": TestDeps(
diff --git a/src/test/regress/expected/remove_cluster_management_nodes.out b/src/test/regress/expected/remove_cluster_management_nodes.out
deleted file mode 100644
index d1af3ed4e..000000000
--- a/src/test/regress/expected/remove_cluster_management_nodes.out
+++ /dev/null
@@ -1,37 +0,0 @@
--- When needed in some tests, GetLocalNodeId() does not behave correctly.
-SELECT citus_remove_node('localhost', 8887);
- citus_remove_node
----------------------------------------------------------------------
-
-(1 row)
-
-SELECT citus_remove_node('localhost', 9995);
- citus_remove_node
----------------------------------------------------------------------
-
-(1 row)
-
-SELECT citus_remove_node('localhost', 9992);
- citus_remove_node
----------------------------------------------------------------------
-
-(1 row)
-
-SELECT citus_remove_node('localhost', 9998);
- citus_remove_node
----------------------------------------------------------------------
-
-(1 row)
-
-SELECT citus_remove_node('localhost', 9997);
- citus_remove_node
----------------------------------------------------------------------
-
-(1 row)
-
-SELECT citus_remove_node('localhost', 8888);
- citus_remove_node
----------------------------------------------------------------------
-
-(1 row)
-
diff --git a/src/test/regress/expected/remove_non_default_nodes.out b/src/test/regress/expected/remove_non_default_nodes.out
new file mode 100644
index 000000000..7645af708
--- /dev/null
+++ b/src/test/regress/expected/remove_non_default_nodes.out
@@ -0,0 +1,13 @@
+-- The default nodes for the citus test suite are coordinator and 2 worker nodes
+-- Which we identify with master_port, worker_1_port, worker_2_port.
+-- When needed in some tests, GetLocalNodeId() does not behave correctly,
+-- So we remove the non default nodes. This tests expects the non default nodes
+-- to not have any active placements.
+SELECT any_value(citus_remove_node('localhost', nodeport))
+FROM pg_dist_node
+WHERE nodeport NOT IN (:master_port, :worker_1_port, :worker_2_port);
+ any_value
+---------------------------------------------------------------------
+
+(1 row)
+
diff --git a/src/test/regress/split_schedule b/src/test/regress/split_schedule
index b46eb22a7..53c422eab 100644
--- a/src/test/regress/split_schedule
+++ b/src/test/regress/split_schedule
@@ -10,7 +10,7 @@ test: foreign_key_to_reference_table
 # Split tests go here.
 test: split_shard
 test: worker_split_copy_test
-test: remove_cluster_management_nodes
+test: remove_non_default_nodes
 test: worker_split_binary_copy_test
 test: worker_split_text_copy_test
 test: citus_split_shard_by_split_points_negative
diff --git a/src/test/regress/sql/remove_cluster_management_nodes.sql b/src/test/regress/sql/remove_cluster_management_nodes.sql
deleted file mode 100644
index 731dd1be0..000000000
--- a/src/test/regress/sql/remove_cluster_management_nodes.sql
+++ /dev/null
@@ -1,7 +0,0 @@
--- When needed in some tests, GetLocalNodeId() does not behave correctly.
-SELECT citus_remove_node('localhost', 8887);
-SELECT citus_remove_node('localhost', 9995);
-SELECT citus_remove_node('localhost', 9992);
-SELECT citus_remove_node('localhost', 9998);
-SELECT citus_remove_node('localhost', 9997);
-SELECT citus_remove_node('localhost', 8888);
diff --git a/src/test/regress/sql/remove_non_default_nodes.sql b/src/test/regress/sql/remove_non_default_nodes.sql
new file mode 100644
index 000000000..4175e87dc
--- /dev/null
+++ b/src/test/regress/sql/remove_non_default_nodes.sql
@@ -0,0 +1,8 @@
+-- The default nodes for the citus test suite are coordinator and 2 worker nodes
+-- Which we identify with master_port, worker_1_port, worker_2_port.
+-- When needed in some tests, GetLocalNodeId() does not behave correctly,
+-- So we remove the non default nodes. This tests expects the non default nodes
+-- to not have any active placements.
+SELECT any_value(citus_remove_node('localhost', nodeport))
+FROM pg_dist_node
+WHERE nodeport NOT IN (:master_port, :worker_1_port, :worker_2_port);