You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/07/22 17:30:09 UTC
[GitHub] [solr] madrob opened a new pull request, #951: SOLR-16304 De-annotate some slow tests
madrob opened a new pull request, #951:
URL: https://github.com/apache/solr/pull/951
Removed Slow annotation from anything that ran faster than 15s on my machine.
Also removed Slow annotation from anything that was already Nightly
https://issues.apache.org/jira/browse/SOLR-16304
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #951:
URL: https://github.com/apache/solr/pull/951#discussion_r933300224
##########
gradle/testing/randomization.gradle:
##########
@@ -85,7 +85,6 @@ allprojects {
[propName: 'tests.timezone', value: "random", description: "Sets the default time zone tests should run with."],
// filtering
[propName: 'tests.filter', value: null, description: "Applies a test filter (see :helpTests)."],
- [propName: 'tests.slow', value: true, description: "Enables or disables @Slow tests."],
Review Comment:
added https://github.com/apache/solr/pull/951/commits/f87fe8ad969e41f8957db3821b1d1593d25f4720 commit, hope that's okay.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] dsmiley commented on pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
dsmiley commented on PR #951:
URL: https://github.com/apache/solr/pull/951#issuecomment-1201993958
Just checking; is "de-annotate some slow tests" still accurate -- thus it is still used but just not on tests that are not sufficiently slow?
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #951:
URL: https://github.com/apache/solr/pull/951#discussion_r933286618
##########
solr/core/src/test/org/apache/solr/security/TestAuthorizationFramework.java:
##########
@@ -52,53 +55,45 @@ public void distribSetUp() throws Exception {
zkStateReader
.getZkClient()
.create(
- ZkStateReader.SOLR_SECURITY_CONF_PATH,
- "{\"authorization\":{\"class\":\"org.apache.solr.security.MockAuthorizationPlugin\"}}"
- .getBytes(StandardCharsets.UTF_8),
- CreateMode.PERSISTENT,
- true);
+ ZkStateReader.SOLR_SECURITY_CONF_PATH, SECURITY_JSON, CreateMode.PERSISTENT, true);
}
}
@Test
public void authorizationFrameworkTest() throws Exception {
MockAuthorizationPlugin.denyUsers.add("user1");
- MockAuthorizationPlugin.denyUsers.add("user1");
- try {
- waitForThingsToLevelOut(10, TimeUnit.SECONDS);
- String baseUrl = jettys.get(0).getBaseUrl().toString();
- verifySecurityStatus(
- ((CloudLegacySolrClient) cloudClient).getHttpClient(),
- baseUrl + "/admin/authorization",
- "authorization/class",
- MockAuthorizationPlugin.class.getName(),
- 20);
- log.info("Starting test");
- ModifiableSolrParams params = new ModifiableSolrParams();
- params.add("q", "*:*");
- // This should work fine.
- cloudClient.query(params);
- MockAuthorizationPlugin.protectedResources.add("/select");
+ waitForThingsToLevelOut(10, TimeUnit.SECONDS);
+ String baseUrl = jettys.get(0).getBaseUrl().toString();
+ verifySecurityStatus(
+ ((CloudLegacySolrClient) cloudClient).getHttpClient(),
+ baseUrl + "/admin/authorization",
+ "authorization/class",
+ s -> MockAuthorizationPlugin.class.getName().equals(s),
+ 20);
+ log.info("Starting test");
Review Comment:
removing logging here too or add back the "Ending test" logging
```suggestion
```
##########
solr/core/src/test/org/apache/solr/rest/TestManagedFileStorage.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.rest;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.rest.ManagedResourceStorage.FileStorageIO;
+import org.apache.solr.rest.ManagedResourceStorage.JsonStorage;
+import org.apache.solr.rest.ManagedResourceStorage.StorageIO;
+import org.junit.Test;
+
+public class TestManagedFileStorage extends SolrTestCaseJ4 {
+
+ /** Runs persisted managed resource creation and update tests on JSON storage. */
+ @Test
+ public void testFileBasedJsonStorage() throws Exception {
+ File instanceDir = createTempDir("json-storage").toFile();
+ try (SolrResourceLoader loader = new SolrResourceLoader(instanceDir.toPath())) {
+ NamedList<String> initArgs = new NamedList<>();
+ String managedDir = instanceDir.getAbsolutePath() + File.separator + "managed";
+ initArgs.add(ManagedResourceStorage.STORAGE_DIR_INIT_ARG, managedDir);
+ FileStorageIO fileStorageIO = new FileStorageIO();
+ fileStorageIO.configure(loader, initArgs);
+ TestManagedFileStorage.doStorageTests(loader, fileStorageIO);
+ }
+ }
+
+ /**
+ * Called from tests for each storage type to run creation and update tests on a persisted managed
+ * resource.
+ */
+ @SuppressWarnings("unchecked")
+ public static void doStorageTests(SolrResourceLoader loader, StorageIO storageIO)
+ throws Exception {
+ String resourceId = "/test/foo";
+
+ JsonStorage jsonStorage = new JsonStorage(storageIO, loader);
+
+ Map<String, String> managedInitArgs = new HashMap<>();
+ managedInitArgs.put("ignoreCase", "true");
+ managedInitArgs.put("dontIgnoreCase", "false");
+
+ List<String> managedList = new ArrayList<>(); // we need a mutable List for this test
+ managedList.addAll(Arrays.asList("a", "b", "c", "d", "e"));
+
+ Map<String, Object> toStore = new HashMap<>();
+ toStore.put(ManagedResource.INIT_ARGS_JSON_FIELD, managedInitArgs);
+ toStore.put(ManagedResource.MANAGED_JSON_LIST_FIELD, managedList);
+
+ jsonStorage.store(resourceId, toStore);
+
+ String storedResourceId = jsonStorage.getStoredResourceId(resourceId);
+ assertTrue(storedResourceId + " file not found!", storageIO.exists(storedResourceId));
+
+ Object fromStorage = jsonStorage.load(resourceId);
+ assertNotNull(fromStorage);
+
+ Map<String, Object> storedMap = (Map<String, Object>) fromStorage;
+ Map<String, Object> storedArgs =
+ (Map<String, Object>) storedMap.get(ManagedResource.INIT_ARGS_JSON_FIELD);
+ assertNotNull(storedArgs);
+ assertEquals("true", storedArgs.get("ignoreCase"));
+ List<String> storedList = (List<String>) storedMap.get(ManagedResource.MANAGED_JSON_LIST_FIELD);
+ assertNotNull(storedList);
+ assertEquals(managedList.size(), storedList.size());
+ assertTrue(storedList.contains("a"));
+
+ // now verify you can update existing data
+ managedInitArgs.put("anotherArg", "someValue");
+ managedList.add("f");
+ jsonStorage.store(resourceId, toStore);
+ fromStorage = jsonStorage.load(resourceId);
+ assertNotNull(fromStorage);
+
+ storedMap = (Map<String, Object>) fromStorage;
+ storedArgs = (Map<String, Object>) storedMap.get(ManagedResource.INIT_ARGS_JSON_FIELD);
+ assertNotNull(storedArgs);
+ assertEquals("someValue", storedArgs.get("anotherArg"));
+ storedList = (List<String>) storedMap.get(ManagedResource.MANAGED_JSON_LIST_FIELD);
+ assertNotNull(storedList);
+ assertEquals(managedList.size(), storedList.size());
+ assertTrue(storedList.contains("e"));
Review Comment:
minor/carry-over from existing code: `f` instead of `e` here since the latter was already previously there and only the former is newly added?
```suggestion
assertTrue(storedList.contains("f"));
```
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
madrob commented on PR #951:
URL: https://github.com/apache/solr/pull/951#issuecomment-1202001079
Slow is no longer used at all
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a diff in pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on code in PR #951:
URL: https://github.com/apache/solr/pull/951#discussion_r927870353
##########
solr/core/src/test/org/apache/solr/cloud/SSLMigrationTest.java:
##########
@@ -49,7 +52,10 @@
@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-12028") // 17-Mar-2018
public class SSLMigrationTest extends AbstractFullDistribZkTestBase {
Review Comment:
unrelated change and `@Slow` annotation retained. accidental commit inclusion probably, it happens.
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] risdenk commented on a diff in pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
risdenk commented on code in PR #951:
URL: https://github.com/apache/solr/pull/951#discussion_r935609307
##########
solr/core/src/test/org/apache/solr/cloud/MigrateRouteKeyTest.java:
##########
@@ -111,7 +109,8 @@ public void multipleShardMigrateTest() throws Exception {
final String splitKey = "a";
final int BIT_SEP = 1;
- final int[] splitKeyCount = new int[1];
+ int splitKeyCount = 0;
+ ;
Review Comment:
Extra `;`
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob merged pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
madrob merged PR #951:
URL: https://github.com/apache/solr/pull/951
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] madrob commented on a diff in pull request #951: SOLR-16304 De-annotate some slow tests
Posted by GitBox <gi...@apache.org>.
madrob commented on code in PR #951:
URL: https://github.com/apache/solr/pull/951#discussion_r927958040
##########
solr/core/src/test/org/apache/solr/cloud/SSLMigrationTest.java:
##########
@@ -49,7 +52,10 @@
@AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-12028") // 17-Mar-2018
public class SSLMigrationTest extends AbstractFullDistribZkTestBase {
Review Comment:
yep, I spent a little bit of time trying to fix this test but didn't finish that
--
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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org