Skip to content

Commit 0d02b5b

Browse files
committed
AVRO-4298: [php] Restore original AVRO_MAX_COLLECTION_ITEMS in tests
Addresses review feedback: the collection-limit tests unconditionally unset AVRO_MAX_COLLECTION_ITEMS in their finally blocks, breaking isolation when the variable is already set in the environment (CI or a developer shell). Capture the pre-existing value in setUp() and restore it via a helper in each finally block instead of blindly unsetting. Assisted-by: GitHub Copilot:claude-opus-4.8
1 parent 2db3fd1 commit 0d02b5b

1 file changed

Lines changed: 34 additions & 12 deletions

File tree

lang/php/test/DatumIOTest.php

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,19 @@
3535

3636
class DatumIOTest extends TestCase
3737
{
38+
/** @var false|string Value of AVRO_MAX_COLLECTION_ITEMS captured before each test. */
39+
private string|false $originalMaxCollectionItems = false;
40+
41+
protected function setUp(): void
42+
{
43+
parent::setUp();
44+
// Capture any pre-existing value so tests that override
45+
// AVRO_MAX_COLLECTION_ITEMS restore it rather than unconditionally
46+
// unsetting it, which would break isolation when the variable is already
47+
// set in the environment (CI or a developer shell).
48+
$this->originalMaxCollectionItems = getenv('AVRO_MAX_COLLECTION_ITEMS');
49+
}
50+
3851
#[DataProvider('data_provider')]
3952
public function test_datum_round_trip(string $schema_json, mixed $datum, string $binary): void
4053
{
@@ -349,7 +362,7 @@ public function test_array_of_null_within_configured_limit_still_reads(): void
349362
$result = $this->decodeWith('{"type":"array","items":"null"}', self::zeroByteBlock(1000));
350363
$this->assertCount(1000, $result);
351364
} finally {
352-
putenv('AVRO_MAX_COLLECTION_ITEMS');
365+
$this->restoreMaxCollectionItems();
353366
}
354367
}
355368

@@ -361,7 +374,7 @@ public function test_array_of_null_exceeds_configured_limit(): void
361374
$this->expectException(AvroIOCollectionSizeException::class);
362375
$this->decodeWith('{"type":"array","items":"null"}', self::zeroByteBlock(1001));
363376
} finally {
364-
putenv('AVRO_MAX_COLLECTION_ITEMS');
377+
$this->restoreMaxCollectionItems();
365378
}
366379
}
367380

@@ -378,7 +391,7 @@ public function test_array_of_null_cumulative_across_blocks(): void
378391
$this->expectException(AvroIOCollectionSizeException::class);
379392
$this->decodeWith('{"type":"array","items":"null"}', $io);
380393
} finally {
381-
putenv('AVRO_MAX_COLLECTION_ITEMS');
394+
$this->restoreMaxCollectionItems();
382395
}
383396
}
384397

@@ -390,7 +403,7 @@ public function test_array_of_null_negative_block_count(): void
390403
$this->expectException(AvroIOCollectionSizeException::class);
391404
$this->decodeWith('{"type":"array","items":"null"}', self::zeroByteBlock(200000, true));
392405
} finally {
393-
putenv('AVRO_MAX_COLLECTION_ITEMS');
406+
$this->restoreMaxCollectionItems();
394407
}
395408
}
396409

@@ -413,7 +426,7 @@ public function test_map_cumulative_limit_not_bypassed_by_duplicate_keys(): void
413426
$this->expectException(AvroIOCollectionSizeException::class);
414427
$this->decodeWith('{"type":"map","values":"null"}', $io);
415428
} finally {
416-
putenv('AVRO_MAX_COLLECTION_ITEMS');
429+
$this->restoreMaxCollectionItems();
417430
}
418431
}
419432

@@ -425,7 +438,7 @@ public function test_array_of_zero_length_fixed_exceeds_limit(): void
425438
$this->expectException(AvroIOCollectionSizeException::class);
426439
$this->decodeWith('{"type":"array","items":{"type":"fixed","name":"empty","size":0}}', self::zeroByteBlock(2000));
427440
} finally {
428-
putenv('AVRO_MAX_COLLECTION_ITEMS');
441+
$this->restoreMaxCollectionItems();
429442
}
430443
}
431444

@@ -440,7 +453,7 @@ public function test_array_of_all_null_record_exceeds_limit(): void
440453
self::zeroByteBlock(2000)
441454
);
442455
} finally {
443-
putenv('AVRO_MAX_COLLECTION_ITEMS');
456+
$this->restoreMaxCollectionItems();
444457
}
445458
}
446459

@@ -462,7 +475,7 @@ public function test_invalid_env_override_falls_back_to_default(): void
462475
$this->expectException(AvroIOCollectionSizeException::class);
463476
$this->decodeWith('{"type":"array","items":"null"}', self::zeroByteBlock(200000000));
464477
} finally {
465-
putenv('AVRO_MAX_COLLECTION_ITEMS');
478+
$this->restoreMaxCollectionItems();
466479
}
467480
}
468481

@@ -475,7 +488,7 @@ public function test_env_override_raises_limit(): void
475488
$result = $this->decodeWith('{"type":"array","items":"null"}', self::zeroByteBlock(15000));
476489
$this->assertCount(15000, $result);
477490
} finally {
478-
putenv('AVRO_MAX_COLLECTION_ITEMS');
491+
$this->restoreMaxCollectionItems();
479492
}
480493
}
481494

@@ -496,7 +509,7 @@ public function test_non_zero_collection_bounded_by_structural_cap(): void
496509
$this->expectException(AvroIOCollectionSizeException::class);
497510
$this->decodeWith('{"type":"array","items":"long"}', $io);
498511
} finally {
499-
putenv('AVRO_MAX_COLLECTION_ITEMS');
512+
$this->restoreMaxCollectionItems();
500513
}
501514
}
502515

@@ -512,7 +525,7 @@ public function test_skip_array_of_null_respects_limit(): void
512525
$this->expectException(AvroIOCollectionSizeException::class);
513526
AvroIODatumReader::skipData($schema, new AvroIOBinaryDecoder($io));
514527
} finally {
515-
putenv('AVRO_MAX_COLLECTION_ITEMS');
528+
$this->restoreMaxCollectionItems();
516529
}
517530
}
518531

@@ -529,7 +542,7 @@ public function test_skip_array_of_null_negative_block_respects_limit(): void
529542
$this->expectException(AvroIOCollectionSizeException::class);
530543
AvroIODatumReader::skipData($schema, new AvroIOBinaryDecoder($io));
531544
} finally {
532-
putenv('AVRO_MAX_COLLECTION_ITEMS');
545+
$this->restoreMaxCollectionItems();
533546
}
534547
}
535548

@@ -692,6 +705,15 @@ public function test_field_default_value(
692705
}
693706
}
694707

708+
protected function restoreMaxCollectionItems(): void
709+
{
710+
if (false === $this->originalMaxCollectionItems) {
711+
putenv('AVRO_MAX_COLLECTION_ITEMS');
712+
} else {
713+
putenv('AVRO_MAX_COLLECTION_ITEMS='.$this->originalMaxCollectionItems);
714+
}
715+
}
716+
695717
/**
696718
* Builds one array/map block of `$count` zero-byte elements plus an end
697719
* marker (a negative count is preceded by a block byte-size).

0 commit comments

Comments
 (0)