Skip to content

Commit 239697d

Browse files
notbakanekonotbakaneko
committed
Revert search score ordering tests. Testing for an exact order is currently too flakey.
Keep the updated query string logic. negative boost should be safe to test... maybe
1 parent 418b94e commit 239697d

5 files changed

Lines changed: 48 additions & 51 deletions

File tree

‎database/factories/BeatmapsetFactory.php‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,6 @@ public function definition(): array
4747
];
4848
}
4949

50-
// Set values that can affect search scoring to a consistent value.
51-
public function consistent()
52-
{
53-
return $this->state(['favourite_count' => 0]);
54-
}
55-
5650
public function deleted()
5751
{
5852
return $this->state(['deleted_at' => now()]);

‎tests/Libraries/Search/BeatmapsetSearch/NegativeBoostTest.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,27 @@
77

88
namespace Tests\Libraries\Search\BeatmapsetSearch;
99

10+
use App\Libraries\Search\BeatmapsetSearch;
11+
use App\Libraries\Search\BeatmapsetSearchRequestParams;
1012
use App\Models\Beatmapset;
13+
use PHPUnit\Framework\Attributes\DataProvider;
1114

1215
class NegativeBoostTest extends TestCase
1316
{
1417
public static function dataProvider(): array
1518
{
1619
return [
17-
[['q' => 'apple'], [2, 1, 0], true],
18-
[['q' => 'apple -too'], [1, 2, 0], true],
19-
[['q' => '-too'], [3, 2, 1, 0], true], // no effect because all the scores are 0
20+
[['q' => 'apple'], [2, 1, 0]],
21+
[['q' => 'apple -too'], [1, 2, 0]],
22+
[['q' => '-too'], [3, 2, 1, 0]], // no effect because all the scores are 0
2023
];
2124
}
2225

2326
public static function setUpBeforeClass(): void
2427
{
2528
static::withDbAccess(function () {
2629
// use something besides empty string so it doesn't match empty string.
27-
$factory = Beatmapset::factory()->consistent()->state(['artist' => 'a', 'creator' => 'a'])->ranked()->withBeatmaps();
30+
$factory = Beatmapset::factory()->state(['artist' => 'a', 'creator' => 'a', 'favourite_count' => 0])->ranked()->withBeatmaps();
2831
static::$beatmapsets = [
2932
$factory->create(['tags' => 'too hoos', 'title' => 'Good Apple']),
3033
$factory->create(['tags' => 'fruit cake', 'title' => 'Apple']),
@@ -34,4 +37,13 @@ public static function setUpBeforeClass(): void
3437
});
3538
parent::setUpBeforeClass();
3639
}
40+
41+
#[DataProvider('dataProvider')]
42+
public function testSearch(array $params, array $expected): void
43+
{
44+
$this->assertEquals(
45+
array_map(fn (int $index) => static::$beatmapsets[$index]->getKey(), $expected),
46+
new BeatmapsetSearch(new BeatmapsetSearchRequestParams($params))->response()->ids()
47+
);
48+
}
3749
}

‎tests/Libraries/Search/BeatmapsetSearch/QueryStringTest.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,17 @@ class QueryStringTest extends TestCase
1313
{
1414
public static function dataProvider(): array
1515
{
16+
// TODO: support scoring order in tests.
1617
return [
1718
[['q' => ''], [0, 1, 2, 3]],
1819
[['q' => '2'], [1]], // id only search.
1920
[['q' => '2 3'], []], // putting more than one id becomes a normal string search.
20-
[['q' => 'triangles'], [0, 1, 3], true],
21+
[['q' => 'triangles'], [0, 1, 3]],
2122
[['q' => '-triangles'], [2]],
22-
[['q' => 'triangles -revival'], [0, 3], true],
23+
[['q' => 'triangles -revival'], [0, 3]],
2324
[['q' => 'cross circle'], [2, 3]],
2425
[['q' => 'cross -circle'], []], // exclusion shouldn't add new matches.
25-
[['q' => 'tri'], [0, 1, 3], true],
26+
[['q' => 'tri'], [0, 1, 3]],
2627
[['q' => 'tri -circ'], [0, 1, 3]],
2728
[['q' => 'tri -circle'], [0, 1, 3]],
2829
[['q' => '-tri'], [0, 1, 2, 3]],
@@ -33,7 +34,7 @@ public static function setUpBeforeClass(): void
3334
{
3435
static::withDbAccess(function () {
3536
// use something besides empty string so it doesn't match empty string.
36-
$factory = Beatmapset::factory()->consistent()->state(['artist' => 'a', 'creator' => 'a'])->ranked()->withBeatmaps();
37+
$factory = Beatmapset::factory()->state(['artist' => 'a', 'creator' => 'a'])->ranked()->withBeatmaps();
3738
static::$beatmapsets = [
3839
$factory->create(['title' => 'Triangles']),
3940
$factory->create(['title' => 'Triangles Revival']),

‎tests/Libraries/Search/BeatmapsetSearch/TestCase.php‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
namespace Tests\Libraries\Search\BeatmapsetSearch;
99

1010
use App\Libraries\Elasticsearch\Es;
11-
use App\Libraries\Elasticsearch\Indexing;
1211
use App\Libraries\Search\BeatmapsetSearch;
1312
use App\Libraries\Search\BeatmapsetSearchParams;
1413
use App\Libraries\Search\BeatmapsetSearchRequestParams;
@@ -27,26 +26,20 @@
2726
abstract class TestCase extends BaseTestCase
2827
{
2928
protected static iterable $beatmapsets = [];
30-
protected static array $existingIndex;
31-
protected static string $testIndex;
3229

3330
abstract public static function dataProvider(): array;
3431

35-
#[AfterClass]
36-
public static function cleanupTestIndex(): void
32+
/**
33+
* Should run before setUpBeforeClass.
34+
* This runs beforeClass as well, to workaround data that may be left in the index from other tests.
35+
* Due to Laravel not sending the transaction events when it wraps tests in a transaction,
36+
* afterCommit may or may not unintentionally index documents,
37+
* depending on whether or no additional transactions are involved in the test.
38+
*/
39+
#[AfterClass, BeforeClass]
40+
public static function cleanupEsBeatmapsets(): void
3741
{
38-
Indexing::updateAlias(Beatmapset::esIndexName(), static::$existingIndex[0]);
39-
Indexing::deleteIndex(static::$testIndex);
40-
}
41-
42-
#[BeforeClass]
43-
public static function createTestIndex(): void
44-
{
45-
// create fresh index for the test so term frequency scoring isn't polluted by previous deleted documents.
46-
static::$existingIndex = Indexing::getOldIndices(Beatmapset::esIndexName());
47-
static::$testIndex = Beatmapset::esIndexName().'_'.snake_case(get_class_basename(get_called_class()));
48-
Beatmapset::esCreateIndex(static::$testIndex);
49-
Indexing::updateAlias(Beatmapset::esIndexName(), static::$testIndex);
42+
new BeatmapsetSearch()->deleteAll();
5043
}
5144

5245
public static function setUpBeforeClass(): void
@@ -56,10 +49,7 @@ public static function setUpBeforeClass(): void
5649
throw new \Exception('No beatmapsets added to test setup.');
5750
}
5851

59-
$indexParams = ['index' => Beatmapset::esIndexName()];
60-
Es::getClient()->indices()->close($indexParams);
61-
Es::getClient()->indices()->open($indexParams);
62-
52+
Es::getClient()->indices()->refresh();
6353
$params = new BeatmapsetSearchParams();
6454
$params->status = 'any';
6555

@@ -84,11 +74,12 @@ public static function tearDownAfterClass(): void
8474
}
8575

8676
#[DataProvider('dataProvider')]
87-
public function testSearch(array $params, array $expected, ?bool $order = null): void
77+
public function testSearch(array $params, array $expected): void
8878
{
89-
$ids = new BeatmapsetSearch(new BeatmapsetSearchRequestParams($params))->response()->ids();
90-
$method = $order ?? false ? 'assertEquals' : 'assertEqualsCanonicalizing';
91-
$this->$method(array_map(fn (int $index) => static::$beatmapsets[$index]->getKey(), $expected), $ids);
79+
$this->assertEqualsCanonicalizing(
80+
array_map(fn (int $index) => static::$beatmapsets[$index]->getKey(), $expected),
81+
new BeatmapsetSearch(new BeatmapsetSearchRequestParams($params))->response()->ids()
82+
);
9283
}
9384

9485
protected function setUp(): void

‎tests/Libraries/Search/BeatmapsetSearch/TitleFilterTest.php‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,30 @@ class TitleFilterTest extends TestCase
1414
public static function dataProvider(): array
1515
{
1616
return [
17-
[['q' => 'best'], [0, 4, 1, 3, 2], true],
18-
[['q' => 'best beatmap'], [1, 3, 2, 0, 4], true],
19-
[['q' => '"best beatmap"'], [1, 3, 2], true],
17+
[['q' => 'best'], [0, 1, 2, 3, 4]],
18+
[['q' => 'best beatmap'], [0, 1, 2, 3, 4]],
19+
[['q' => '"best beatmap"'], [1, 2, 3]],
2020
[['q' => '-best'], []],
2121
[['q' => '-best -beatmap'], []],
22-
[['q' => '-"best beatmap"'], [4, 0], true],
22+
[['q' => '-"best beatmap"'], [0, 4]],
2323

24-
[['q' => 'title=best'], [0, 1, 3, 2], true],
25-
[['q' => 'title="best beatmap"'], [1, 3, 2], true],
26-
[['q' => 'title="the beatmap"'], [2, 3], true],
27-
[['q' => 'title=""best beatmap""'], [1, 3, 2], true],
24+
[['q' => 'title=best'], [0, 1, 2, 3]],
25+
[['q' => 'title="best beatmap"'], [1, 2, 3]],
26+
[['q' => 'title="the beatmap"'], [1, 2]],
27+
[['q' => 'title=""best beatmap""'], [1, 2, 3]],
2828
[['q' => 'title=""the beatmap""'], []],
2929
];
3030
}
3131

3232
public static function setUpBeforeClass(): void
3333
{
3434
static::withDbAccess(function () {
35-
$factory = Beatmapset::factory()->consistent()->ranked()->withBeatmaps();
35+
$factory = Beatmapset::factory()->ranked()->withBeatmaps();
3636
static::$beatmapsets = [
3737
$factory->create(['title' => 'best']),
38-
$factory->create(['title' => 'best beatmap']),
3938
$factory->create(['title' => 'the best beatmap']),
40-
$factory->create(['title' => 'the best beatmap', 'title_unicode' => 'ダ best beatmap']), // scores slightly better in prefix matching.
41-
// this will score better in the 'best' test due to the term having lower frequency in the artist field.
39+
$factory->create(['title' => 'the best beatmap', 'title_unicode' => 'ダ best beatmap']),
40+
$factory->create(['title' => 'best beatmap']),
4241
$factory->create(['artist' => 'the best artist']),
4342
];
4443
});

0 commit comments

Comments
 (0)