Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport Main] Add path.repo for registering snapshot repository in integ tests #2463

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ integTest {
systemProperty 'tests.security.manager', 'false'
systemProperty 'java.io.tmpdir', opensearch_tmp_dir.absolutePath
systemProperty "java.library.path", "$rootDir/jni/release"
systemProperty "tests.path.repo", "${buildDir}/testSnapshotFolder"
// allows integration test classes to access test resource from project root path
systemProperty('project.root', project.rootDir.absolutePath)

Expand Down Expand Up @@ -448,6 +449,9 @@ testClusters.integTest {
}
}
systemProperty("java.library.path", "$rootDir/jni/release")
final testSnapshotFolder = file("${buildDir}/testSnapshotFolder")
testSnapshotFolder.mkdirs()
setting 'path.repo', "${buildDir}/testSnapshotFolder"
systemProperty propertyKeys.breaker.useRealMemory, getBreakerSetting()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class ModelIT extends AbstractRestartUpgradeTestCase {
private static final int DELAY_MILLI_SEC = 1000;
private static final int MIN_NUM_OF_MODELS = 2;
private static final int K = 5;
private static final int NUM_DOCS = 10;
private static final int NUM_DOCS = 1100;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed ? Don't see any correlation with this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is related to another backport PR #2443 about updating error messages for faiss training. That other backport PR made the same fix as above, but it hasn't been merged yet, which is why I made this change in this PR to get the tests passing

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. We should not make it as part of this PR then and let other PR merge, then rebase this PR on top of it before merging.

private static final int NUM_DOCS_TEST_MODEL_INDEX = 100;
private static final int NUM_DOCS_TEST_MODEL_INDEX_DEFAULT = 100;
private static final int NUM_DOCS_TEST_MODEL_INDEX_FOR_NON_KNN_INDEX = 100;
Expand Down
15 changes: 15 additions & 0 deletions src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java
Original file line number Diff line number Diff line change
Expand Up @@ -1980,4 +1980,19 @@ protected boolean isApproximateThresholdSupported(final Optional<String> bwcVers
protected static String randomLowerCaseString() {
return randomAlphaOfLengthBetween(MIN_CODE_UNITS, MAX_CODE_UNITS).toLowerCase(Locale.ROOT);
}

@SneakyThrows
protected void setupSnapshotRestore(String index, String snapshot, String repository) {
final String pathRepo = System.getProperty("tests.path.repo");

// create index
createIndex(index, getDefaultIndexSettings());

// create repo
Settings repoSettings = Settings.builder().put("compress", randomBoolean()).put("location", pathRepo).build();
registerRepository(repository, "fs", true, repoSettings);

// create snapshot
createSnapshot(repository, snapshot, true);
}
}
Loading