Skip to content

Commit d7af0d4

Browse files
committed
fix test failure
Signed-off-by: Xue Zhou <[email protected]>
1 parent 1f47ec5 commit d7af0d4

File tree

17 files changed

+106
-98
lines changed

17 files changed

+106
-98
lines changed

plugins/repository-azure/src/test/java/org/opensearch/repositories/azure/AzureRepositorySettingsTests.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
package org.opensearch.repositories.azure;
3434

35-
import org.opensearch.common.settings.SettingsException;
3635
import reactor.core.scheduler.Schedulers;
3736

3837
import org.junit.AfterClass;
@@ -47,7 +46,6 @@
4746
import org.opensearch.repositories.blobstore.BlobStoreTestUtil;
4847
import org.opensearch.test.OpenSearchTestCase;
4948

50-
import static org.hamcrest.Matchers.instanceOf;
5149
import static org.hamcrest.Matchers.is;
5250
import static org.hamcrest.Matchers.nullValue;
5351
import static org.mockito.Mockito.mock;
@@ -163,22 +161,21 @@ public void testChunkSize() {
163161
assertEquals(new ByteSizeValue(size, ByteSizeUnit.MB), azureRepository.chunkSize());
164162

165163
// zero bytes is not allowed
166-
SettingsException e = expectThrows(
167-
SettingsException.class,
164+
IllegalArgumentException e = expectThrows(
165+
IllegalArgumentException.class,
168166
() -> azureRepository(Settings.builder().put("chunk_size", "0").build())
169167
);
170-
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
171-
assertEquals("failed to parse value [0] for setting [chunk_size], must be >= [1b]", e.getCause().getMessage());
168+
assertEquals("failed to parse value [0] for setting [chunk_size], must be >= [1b]", e.getMessage());
172169

173170
// negative bytes not allowed
174-
e = expectThrows(SettingsException.class, () -> azureRepository(Settings.builder().put("chunk_size", "-1").build()));
175-
assertEquals("failed to parse value [-1] for setting [chunk_size], must be >= [1b]", e.getCause().getMessage());
171+
e = expectThrows(IllegalArgumentException.class, () -> azureRepository(Settings.builder().put("chunk_size", "-1").build()));
172+
assertEquals("failed to parse value [-1] for setting [chunk_size], must be >= [1b]", e.getMessage());
176173

177174
// greater than max chunk size not allowed
178-
e = expectThrows(SettingsException.class, () -> azureRepository(Settings.builder().put("chunk_size", "6tb").build()));
175+
e = expectThrows(IllegalArgumentException.class, () -> azureRepository(Settings.builder().put("chunk_size", "6tb").build()));
179176
assertEquals(
180177
"failed to parse value [6tb] for setting [chunk_size], must be <= [" + AzureStorageService.MAX_CHUNK_SIZE.getStringRep() + "]",
181-
e.getCause().getMessage()
178+
e.getMessage()
182179
);
183180
}
184181

plugins/repository-gcs/src/internalClusterTest/java/org/opensearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import org.opensearch.common.regex.Regex;
5656
import org.opensearch.common.settings.MockSecureSettings;
5757
import org.opensearch.common.settings.Settings;
58+
import org.opensearch.common.settings.SettingsException;
5859
import org.opensearch.common.unit.ByteSizeUnit;
5960
import org.opensearch.common.unit.ByteSizeValue;
6061
import org.opensearch.common.xcontent.NamedXContentRegistry;
@@ -163,37 +164,37 @@ public void testChunkSize() {
163164
assertEquals(new ByteSizeValue(size, ByteSizeUnit.MB), chunkSize);
164165

165166
// zero bytes is not allowed
166-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
167+
SettingsException e = expectThrows(SettingsException.class, () -> {
167168
final RepositoryMetadata repoMetadata = new RepositoryMetadata(
168169
"repo",
169170
GoogleCloudStorageRepository.TYPE,
170171
Settings.builder().put("chunk_size", "0").build()
171172
);
172173
GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetadata);
173174
});
174-
assertEquals("failed to parse value [0] for setting [chunk_size], must be >= [1b]", e.getMessage());
175+
assertEquals("failed to parse value [0] for setting [chunk_size], must be >= [1b]", e.getCause().getMessage());
175176

176177
// negative bytes not allowed
177-
e = expectThrows(IllegalArgumentException.class, () -> {
178+
e = expectThrows(SettingsException.class, () -> {
178179
final RepositoryMetadata repoMetadata = new RepositoryMetadata(
179180
"repo",
180181
GoogleCloudStorageRepository.TYPE,
181182
Settings.builder().put("chunk_size", "-1").build()
182183
);
183184
GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetadata);
184185
});
185-
assertEquals("failed to parse value [-1] for setting [chunk_size], must be >= [1b]", e.getMessage());
186+
assertEquals("failed to parse value [-1] for setting [chunk_size], must be >= [1b]", e.getCause().getMessage());
186187

187188
// greater than max chunk size not allowed
188-
e = expectThrows(IllegalArgumentException.class, () -> {
189+
e = expectThrows(SettingsException.class, () -> {
189190
final RepositoryMetadata repoMetadata = new RepositoryMetadata(
190191
"repo",
191192
GoogleCloudStorageRepository.TYPE,
192193
Settings.builder().put("chunk_size", "6tb").build()
193194
);
194195
GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetadata);
195196
});
196-
assertEquals("failed to parse value [6tb] for setting [chunk_size], must be <= [5tb]", e.getMessage());
197+
assertEquals("failed to parse value [6tb] for setting [chunk_size], must be <= [5tb]", e.getCause().getMessage());
197198
}
198199

199200
public void testWriteReadLarge() throws IOException {

plugins/repository-s3/src/test/java/org/opensearch/repositories/s3/S3RepositoryTests.java

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import org.opensearch.cluster.metadata.RepositoryMetadata;
3737
import org.opensearch.common.settings.ClusterSettings;
3838
import org.opensearch.common.settings.Settings;
39-
import org.opensearch.common.settings.SettingsException;
4039
import org.opensearch.common.unit.ByteSizeUnit;
4140
import org.opensearch.common.unit.ByteSizeValue;
4241
import org.opensearch.common.xcontent.NamedXContentRegistry;
@@ -89,16 +88,17 @@ public void testInvalidChunkBufferSizeSettings() {
8988
createS3Repo(getRepositoryMetadata(s3)).close();
9089
// buffer < 5mb should fail
9190
final Settings s4 = bufferAndChunkSettings(4, 10);
92-
final SettingsException e2 = expectThrows(SettingsException.class, () -> createS3Repo(getRepositoryMetadata(s4)).close());
93-
assertThat(e2.getCause(), Matchers.instanceOf(IllegalArgumentException.class));
94-
assertThat(e2.getCause().getMessage(), containsString("failed to parse value [4mb] for setting [buffer_size], must be >= [5mb]"));
91+
final IllegalArgumentException e2 = expectThrows(
92+
IllegalArgumentException.class,
93+
() -> createS3Repo(getRepositoryMetadata(s4)).close()
94+
);
95+
assertThat(e2.getMessage(), containsString("failed to parse value [4mb] for setting [buffer_size], must be >= [5mb]"));
9596
final Settings s5 = bufferAndChunkSettings(5, 6000000);
96-
final SettingsException e3 = expectThrows(SettingsException.class, () -> createS3Repo(getRepositoryMetadata(s5)).close());
97-
assertThat(e3.getCause(), Matchers.instanceOf(IllegalArgumentException.class));
98-
assertThat(
99-
e3.getCause().getMessage(),
100-
containsString("failed to parse value [6000000mb] for setting [chunk_size], must be <= [5tb]")
97+
final IllegalArgumentException e3 = expectThrows(
98+
IllegalArgumentException.class,
99+
() -> createS3Repo(getRepositoryMetadata(s5)).close()
101100
);
101+
assertThat(e3.getMessage(), containsString("failed to parse value [6000000mb] for setting [chunk_size], must be <= [5tb]"));
102102
}
103103

104104
private Settings bufferAndChunkSettings(long buffer, long chunk) {

qa/evil-tests/src/test/java/org/opensearch/cluster/metadata/EvilSystemPropertyTests.java

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,20 @@ public class EvilSystemPropertyTests extends OpenSearchTestCase {
4545

4646
@SuppressForbidden(reason = "manipulates system properties for testing")
4747
public void testNumShards() {
48-
SettingsException exception = expectThrows(SettingsException.class, () ->
48+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () ->
4949
IndexMetadata.buildNumberOfShardsSetting()
5050
.get(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1025).build()));
51-
assertEquals("Failed to parse value [1025] for setting [" + SETTING_NUMBER_OF_SHARDS + "] must be <= 1024", exception.getCause().getMessage());
51+
assertEquals("Failed to parse value [1025] for setting [" + SETTING_NUMBER_OF_SHARDS + "] must be <= 1024", exception.getMessage());
5252

5353
Integer numShards = IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.get(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 100).build());
5454
assertEquals(100, numShards.intValue());
5555
int limit = randomIntBetween(1, 10);
5656
System.setProperty(MAX_NUMBER_OF_SHARDS, Integer.toString(limit));
5757
try {
58-
SettingsException e = expectThrows(SettingsException.class, () ->
58+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
5959
IndexMetadata.buildNumberOfShardsSetting()
6060
.get(Settings.builder().put("index.number_of_shards", 11).build()));
61-
Throwable cause = e.getCause();
62-
assertThat(cause, Matchers.instanceOf(IllegalArgumentException.class));
63-
assertEquals("Failed to parse value [11] for setting [index.number_of_shards] must be <= " + limit, cause.getMessage());
61+
assertEquals("Failed to parse value [11] for setting [index.number_of_shards] must be <= " + limit, e.getMessage());
6462
System.clearProperty(MAX_NUMBER_OF_SHARDS);
6563

6664
Integer defaultFromSetting = IndexMetadata.INDEX_NUMBER_OF_SHARDS_SETTING.getDefault(Settings.EMPTY);
@@ -74,10 +72,9 @@ public void testNumShards() {
7472
randomDefault = randomIntBetween(1, 10);
7573
System.setProperty(MAX_NUMBER_OF_SHARDS, Integer.toString(randomDefault));
7674
System.setProperty(DEFAULT_NUMBER_OF_SHARDS, Integer.toString(randomDefault + 1));
77-
78-
cause = expectThrows(IllegalArgumentException.class, IndexMetadata::buildNumberOfShardsSetting);
75+
e = expectThrows(IllegalArgumentException.class, IndexMetadata::buildNumberOfShardsSetting);
7976
assertEquals(DEFAULT_NUMBER_OF_SHARDS + " value [" + (randomDefault + 1) + "] must between " +
80-
"1 and " + MAX_NUMBER_OF_SHARDS + " [" + randomDefault + "]", cause.getMessage());
77+
"1 and " + MAX_NUMBER_OF_SHARDS + " [" + randomDefault + "]", e.getMessage());
8178
} finally {
8279
System.clearProperty(MAX_NUMBER_OF_SHARDS);
8380
System.clearProperty(DEFAULT_NUMBER_OF_SHARDS);

qa/multi-cluster-search/src/test/resources/rest-api-spec/test/multi_cluster/15_connection_mode_configuration.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
cluster.remote.test_remote_cluster.proxy_address: $remote_ip
1818

1919
- match: { status: 400 }
20-
- match: { error.root_cause.0.type: "settings_exception" }
20+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
2121
- match: { error.root_cause.0.reason: "Setting \"cluster.remote.test_remote_cluster.node_connections\" cannot be
2222
used with the configured \"cluster.remote.test_remote_cluster.mode\" [required=SNIFF, configured=PROXY]" }
2323

@@ -32,7 +32,7 @@
3232
cluster.remote.test_remote_cluster.proxy_address: $remote_ip
3333

3434
- match: { status: 400 }
35-
- match: { error.root_cause.0.type: "settings_exception" }
35+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
3636
- match: { error.root_cause.0.reason: "Setting \"cluster.remote.test_remote_cluster.seeds\" cannot be
3737
used with the configured \"cluster.remote.test_remote_cluster.mode\" [required=SNIFF, configured=PROXY]" }
3838

@@ -54,7 +54,7 @@
5454
cluster.remote.test_remote_cluster.seeds: $remote_ip
5555

5656
- match: { status: 400 }
57-
- match: { error.root_cause.0.type: "settings_exception" }
57+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
5858
- match: { error.root_cause.0.reason: "Setting \"cluster.remote.test_remote_cluster.proxy_socket_connections\" cannot be
5959
used with the configured \"cluster.remote.test_remote_cluster.mode\" [required=PROXY, configured=SNIFF]" }
6060

@@ -68,7 +68,7 @@
6868
cluster.remote.test_remote_cluster.seeds: $remote_ip
6969

7070
- match: { status: 400 }
71-
- match: { error.root_cause.0.type: "settings_exception" }
71+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
7272
- match: { error.root_cause.0.reason: "Setting \"cluster.remote.test_remote_cluster.proxy_address\" cannot be
7373
used with the configured \"cluster.remote.test_remote_cluster.mode\" [required=PROXY, configured=SNIFF]" }
7474

@@ -182,7 +182,7 @@
182182
cluster.remote.test_remote_cluster.proxy_address: $remote_ip
183183

184184
- match: { status: 400 }
185-
- match: { error.root_cause.0.type: "settings_exception" }
185+
- match: { error.root_cause.0.type: "illegal_argument_exception" }
186186
- match: { error.root_cause.0.reason: "Setting \"cluster.remote.test_remote_cluster.seeds\" cannot be
187187
used with the configured \"cluster.remote.test_remote_cluster.mode\" [required=SNIFF, configured=PROXY]" }
188188

server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import org.opensearch.common.regex.Regex;
6161
import org.opensearch.common.settings.IndexScopedSettings;
6262
import org.opensearch.common.settings.Settings;
63-
import org.opensearch.common.settings.SettingsException;
6463
import org.opensearch.common.unit.TimeValue;
6564
import org.opensearch.common.util.set.Sets;
6665
import org.opensearch.common.xcontent.NamedXContentRegistry;
@@ -253,7 +252,7 @@ ClusterState addComponentTemplate(
253252
) throws Exception {
254253
final ComponentTemplate existing = currentState.metadata().componentTemplates().get(name);
255254
if (create && existing != null) {
256-
throw new SettingsException("component template [" + name + "] already exists");
255+
throw new IllegalArgumentException("component template [" + name + "] already exists");
257256
}
258257

259258
CompressedXContent mappings = template.template().mappings();
@@ -289,7 +288,7 @@ ClusterState addComponentTemplate(
289288
}
290289
}
291290
if (globalTemplatesThatUseThisComponent.isEmpty() == false) {
292-
throw new SettingsException(
291+
throw new IllegalArgumentException(
293292
"cannot update component template ["
294293
+ name
295294
+ "] because the following global templates would resolve to specifying the ["

server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
202202
Map<String, Settings> groups = s.getAsGroups();
203203
for (String key : SimilarityService.BUILT_IN.keySet()) {
204204
if (groups.containsKey(key)) {
205-
throw new SettingsException(
205+
throw new IllegalArgumentException(
206206
"illegal value for [index.similarity." + key + "] cannot redefine built-in similarity"
207207
);
208208
}
@@ -254,7 +254,7 @@ public IndexScopedSettings copy(Settings settings, IndexMetadata metadata) {
254254
@Override
255255
protected void validateSettingKey(Setting setting) {
256256
if (setting.getKey().startsWith("index.") == false) {
257-
throw new SettingsException("illegal settings key: [" + setting.getKey() + "] must start with [index.]");
257+
throw new IllegalArgumentException("illegal settings key: [" + setting.getKey() + "] must start with [index.]");
258258
}
259259
super.validateSettingKey(setting);
260260
}

server/src/main/java/org/opensearch/common/settings/Setting.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,15 @@ private T get(Settings settings, boolean validate) {
496496
}
497497
return parsed;
498498
} catch (OpenSearchParseException ex) {
499-
throw new SettingsException(ex.getMessage(), ex);
500-
} catch (Exception ex) {
499+
throw new IllegalArgumentException(ex.getMessage(), ex);
500+
} catch (NumberFormatException ex) {
501501
String err = "Failed to parse value" + (isFiltered() ? "" : " [" + value + "]") + " for setting [" + getKey() + "]";
502-
throw new SettingsException(err, ex);
502+
throw new IllegalArgumentException(err, ex);
503+
} catch (IllegalArgumentException ex) {
504+
throw ex;
505+
} catch (Exception t) {
506+
String err = "Failed to parse value" + (isFiltered() ? "" : " [" + value + "]") + " for setting [" + getKey() + "]";
507+
throw new IllegalArgumentException(err, t);
503508
}
504509
}
505510

server/src/test/java/org/opensearch/action/admin/cluster/settings/SettingsUpdaterTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.opensearch.common.settings.Setting;
4040
import org.opensearch.common.settings.Setting.Property;
4141
import org.opensearch.common.settings.Settings;
42+
import org.opensearch.common.settings.SettingsException;
4243
import org.opensearch.test.OpenSearchTestCase;
4344

4445
import java.util.ArrayList;
@@ -157,7 +158,7 @@ public void testAllOrNothing() {
157158
logger
158159
);
159160
fail("all or nothing");
160-
} catch (IllegalArgumentException ex) {
161+
} catch (SettingsException ex) {
161162
logger.info("", ex);
162163
assertEquals("Failed to parse value [not a float] for setting [cluster.routing.allocation.balance.index]", ex.getMessage());
163164
}
@@ -663,11 +664,11 @@ public void testUpdateOfValidationDependentSettings() {
663664

664665
final ClusterState finalCluster = cluster;
665666
Exception exception = expectThrows(
666-
IllegalArgumentException.class,
667+
SettingsException.class,
667668
() -> updater.updateSettings(finalCluster, Settings.builder().put(SETTING_FOO_HIGH.getKey(), 2).build(), Settings.EMPTY, logger)
668669
);
669670

670-
assertThat(exception.getMessage(), equalTo("[high]=2 is lower than [low]=5"));
671+
assertThat(exception.getCause().getMessage(), equalTo("[high]=2 is lower than [low]=5"));
671672
}
672673

673674
}

server/src/test/java/org/opensearch/action/support/AutoCreateIndexTests.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.opensearch.common.collect.Tuple;
4242
import org.opensearch.common.settings.ClusterSettings;
4343
import org.opensearch.common.settings.Settings;
44+
import org.opensearch.common.settings.SettingsException;
4445
import org.opensearch.common.util.concurrent.ThreadContext;
4546
import org.opensearch.index.IndexNotFoundException;
4647
import org.opensearch.indices.SystemIndexDescriptor;
@@ -62,11 +63,11 @@ public void testParseFailed() {
6263
Settings settings = Settings.builder().put("action.auto_create_index", ",,,").build();
6364
newAutoCreateIndex(settings);
6465
fail("initialization should have failed");
65-
} catch (IllegalArgumentException ex) {
66+
} catch (SettingsException ex) {
6667
assertEquals(
6768
"Can't parse [,,,] for setting [action.auto_create_index] must be either [true, false, or a "
6869
+ "comma separated list of index patterns]",
69-
ex.getMessage()
70+
ex.getCause().getMessage()
7071
);
7172
}
7273
}
@@ -77,10 +78,10 @@ public void testParseFailedMissingIndex() {
7778
try {
7879
newAutoCreateIndex(settings);
7980
fail("initialization should have failed");
80-
} catch (IllegalArgumentException ex) {
81+
} catch (SettingsException ex) {
8182
assertEquals(
8283
"Can't parse [" + prefix + "] for setting [action.auto_create_index] must contain an index name after [" + prefix + "]",
83-
ex.getMessage()
84+
ex.getCause().getMessage()
8485
);
8586
}
8687
}

0 commit comments

Comments
 (0)