Skip to content

Commit 9708fd9

Browse files
test: remove obsolete Python runner and duplicate categorical passes (#2735)
* test: remove obsolete Python runner and duplicate categorical passes ## Summary Delete the unreferenced Scala 2.11 Python test runner and two identical categorical test repetitions. Clean up imports from the previously removed Maps Spatial suite and preserve retired-stage serialization contracts offline. ## Prompting Intent Audit outdated or deprecated SynapseML tests in an isolated worktree and remove only coverage proven obsolete or redundant, separately from CI path selection. ## Linked Sources - User request in this session for two independent test-efficiency PRs. - #2485 - project/CodegenPlugin.scala and core codegen test sources at 714d365. - https://learn.microsoft.com/en-us/azure/ai-services/document-intelligence/overview?view=doc-intel-4.0.0 - Committed test-retirement review and audit decisions under reviews/. ## Rationale The old runner has no tracked caller and points at a superseded Scala version and test namespace. The removed loop variables never influence either body. Retain all named tests and assertions, genuine metadata variants, and supported legacy API coverage. No new retired-service suite was established for removal. The public retired Maps stage remains serializable, so add credential-free round-trip coverage rather than deleting its compatibility tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: group test-retirement review under its PR number ## Summary Move the existing review report into reviews/pr-2735 with byte-identical content. ## Prompting Intent The user requested that review artifacts live in reviews/pr-<pr_number>. The corresponding repository-wide instruction is included in companion PR2736. ## Linked Sources - #2735 - #2736 - User instruction: reviews should be in reviews/pr-<pr_number>. ## Rationale Group review evidence by the PR it describes while retaining its original filename and historical content. Leave the GeospatialCoreSuite tests unchanged; the user asked for their rationale, not for their removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 714d365 commit 9708fd9

5 files changed

Lines changed: 109 additions & 37 deletions

File tree

‎cognitive/src/test/scala/com/microsoft/azure/synapse/ml/services/geospatial/AzureMapsSuite.scala‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,12 @@
44
package com.microsoft.azure.synapse.ml.services.geospatial
55

66
import com.microsoft.azure.synapse.ml.Secrets
7-
import com.microsoft.azure.synapse.ml.build.BuildInfo
8-
import com.microsoft.azure.synapse.ml.services.URLEncodingUtils
97
import com.microsoft.azure.synapse.ml.core.test.fuzzing.{TestObject, TransformerFuzzing}
10-
import com.microsoft.azure.synapse.ml.services.geospatial.AzureMapsJsonProtocol._
11-
import com.microsoft.azure.synapse.ml.io.http.{HeaderValues, RESTHelpers}
128
import com.microsoft.azure.synapse.ml.stages.{FixedMiniBatchTransformer, FlattenBatch}
13-
import org.apache.http.client.methods.{HttpDelete, HttpGet, HttpPost}
14-
import org.apache.http.entity.StringEntity
159
import org.apache.spark.ml.util.MLReadable
1610
import org.apache.spark.sql.DataFrame
1711
import org.apache.spark.sql.functions.col
1812

19-
import java.net.URI
20-
2113
trait AzureMapsKey {
2214
lazy val azureMapsKey: String = sys.env.getOrElse("AZURE_MAPS_KEY", Secrets.AzureMapsKey)
2315
}
@@ -158,4 +150,5 @@ class AzMapsSearchReverseAddressSuite extends TransformerFuzzing[ReverseAddressG
158150
// AzMapsPointInPolygonSuite was removed because the Azure Maps Spatial service
159151
// was retired on September 30, 2025. The CheckPointInPolygon transformer now
160152
// throws UnsupportedOperationException on transform().
153+
// Keep its offline request, schema, retirement-error, and persistence coverage in GeospatialCoreSuite.
161154
// See: https://azure.microsoft.com/en-us/updates/v2/azure-maps-creator-services-retirement-on-30-september-2025

‎cognitive/src/test/scala/com/microsoft/azure/synapse/ml/services/geospatial/GeospatialCoreSuite.scala‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,40 @@ class GeospatialCoreSuite extends TestBase {
164164
}
165165
assert(retiredError.getMessage.contains("retired on September 30, 2025"))
166166
}
167+
168+
test("retired checkpoint stage preserves scalar and column parameters through save and load") {
169+
val input = Seq((Seq(47.6418), Seq(-122.1275), "udid-1")).toDF("latitude", "longitude", "udid")
170+
val scalar = new CheckPointInPolygon()
171+
.setLatitude(47.6418)
172+
.setLongitude(-122.1275)
173+
.setUserDataIdentifier("udid-1")
174+
val columns = new CheckPointInPolygon()
175+
.setLatitudeCol("latitude")
176+
.setLongitudeCol("longitude")
177+
.setUserDataIdentifierCol("udid")
178+
179+
Seq(scalar, columns).zipWithIndex.foreach { case (stage, index) =>
180+
stage.setSubscriptionKey("fake-key")
181+
.setGeography("us")
182+
.setOutputCol("pointInPolygon")
183+
.setErrorCol("pointInPolygonError")
184+
val expectedSchema = stage.transformSchema(input.schema)
185+
val path = tmpDir.resolve(s"checkpoint-$index").toString
186+
stage.write.save(path)
187+
val loaded = CheckPointInPolygon.load(path)
188+
189+
assert(loaded.uid == stage.uid)
190+
assert(loaded.getUrl == stage.getUrl)
191+
assert(loaded.getSubscriptionKey == "fake-key")
192+
assert(loaded.getOrDefault(loaded.latitude) == stage.getOrDefault(stage.latitude))
193+
assert(loaded.getOrDefault(loaded.longitude) == stage.getOrDefault(stage.longitude))
194+
assert(loaded.getOrDefault(loaded.userDataIdentifier) == stage.getOrDefault(stage.userDataIdentifier))
195+
assert(loaded.transformSchema(input.schema) == expectedSchema)
196+
197+
val error = intercept[UnsupportedOperationException] {
198+
loaded.transform(input)
199+
}
200+
assert(error.getMessage.contains("retired on September 30, 2025"))
201+
}
202+
}
167203
}

‎core/src/test/scala/com/microsoft/azure/synapse/ml/featurize/VerifyValueIndexer.scala‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,8 @@ class VerifyIndexToValue extends ValueIndexerUtilities with TransformerFuzzing[I
4040
private val testName = col + "_noncat"
4141

4242
test("Test: Going to Categorical and Back") {
43-
for (mmlStyle <- List(false, true)) { // TODO this is not used?
44-
val df1 = new IndexToValue().setInputCol(newName).setOutputCol(testName).transform(df2)
45-
df1.select(col, testName).collect.foreach(row => assert(row(0) == row(1), "two columns should be the same"))
46-
}
43+
val df1 = new IndexToValue().setInputCol(newName).setOutputCol(testName).transform(df2)
44+
df1.select(col, testName).collect.foreach(row => assert(row(0) == row(1), "two columns should be the same"))
4745
}
4846

4947
override def testObjects(): scala.Seq[TestObject[IndexToValue]] = Seq(new TestObject(
@@ -79,17 +77,15 @@ class VerifyValueIndexer extends ValueIndexerUtilities with EstimatorFuzzing[Val
7977
val col = "string"
8078
val trueLevels = df.select("string").collect().map(_(0).toString).distinct.sorted
8179

82-
for (mmlStyle <- List(false, true)) { // TODO this is not used?
83-
val newName = col + "_cat"
84-
val df2 = new ValueIndexer().setInputCol(col).setOutputCol(newName).fit(df).transform(df)
80+
val newName = col + "_cat"
81+
val df2 = new ValueIndexer().setInputCol(col).setOutputCol(newName).fit(df).transform(df)
8582

86-
val map = CategoricalUtilities.getMap[String](df2.schema(newName).metadata)
83+
val map = CategoricalUtilities.getMap[String](df2.schema(newName).metadata)
8784

88-
val levels = map.levels.sorted
85+
val levels = map.levels.sorted
8986

90-
(trueLevels zip levels).foreach {
91-
case (a, b) => assert(a == b, "categorical levels are not the same")
92-
}
87+
(trueLevels zip levels).foreach {
88+
case (a, b) => assert(a == b, "categorical levels are not the same")
9389
}
9490
}
9591

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Test retirement review
2+
3+
Reviewed the agent's exact four-file patch and evidence report against
4+
`714d365e71f6d2db5b7072094a4a3ad22485eb57`. No unresolved finding.
5+
6+
## Removal decisions
7+
8+
| Candidate | Decision and evidence |
9+
| --- | --- |
10+
| `tools/pytest/run_all_tests.py` | Remove. Its Scala 2.11 output path and `synapseml` test namespace do not match the current build. Tracked-text caller search found no caller. `CodegenPlugin.testPython` already invokes pytest/JUnit against `synapsemltest`; that replacement exists even at the available February 2023 history boundary. No current tests or dependency pins are removed. |
11+
| Two `mmlStyle` loops in `VerifyValueIndexer.scala` | Remove only the loop wrappers. Neither body reads the loop variable, so both iterations have identical inputs and assertions. Both named tests, every assertion, inherited fuzzing, and the genuine metadata-style test in `TestCategoricals.scala` remain. |
12+
| Azure Maps Spatial live tests | Already removed by microsoft/SynapseML#2485. Remove only orphaned imports now; retain address-geocoder suites and offline retired-stage contracts. Add scalar/column save-load coverage because the retired stage cannot use the ordinary live serialization fuzzer. |
13+
| Form Recognizer v2.1, Text Analytics, legacy OCR, OpenAI deprecated aliases | Retain. Deprecated public APIs and compatibility stubs still need contract coverage. No already-passed retirement was established for these remaining live endpoints. |
14+
| Ignored null/NaN, performance, slow service and model tests | Retain. Ignored, slow, or old does not establish redundancy or obsolescence. |
15+
16+
The audit covered 396 tracked test-source/helper files and examined the unused
17+
runner separately. It found no nonempty whole-file duplicates. This is not a
18+
claim of complete semantic deduplication.
19+
20+
## Review evidence
21+
22+
- Correctness: the categorical edit only removes duplicate execution. No named
23+
test or assertion is deleted; the old ignored null/NaN case remains.
24+
- Architecture and compatibility: no production class, public signature,
25+
dependency, generated file, or active runner changes. Offline Maps coverage
26+
protects the retained reader and serialized scalar/column parameter shapes.
27+
- Edge cases: persistence checks UID, endpoint, fake subscription key,
28+
coordinates/UDID, output/error schema, and the explicit retirement exception
29+
after load. Both scalar and column-bound instances are exercised.
30+
- Resource safety: the added test uses the suite's managed temporary directory
31+
and cannot send HTTP because the retired transform throws immediately.
32+
- Validation: baseline categorical tests passed before the edit. Final affected
33+
main/Test compilation and scalastyle passed under JDK 11. Twenty-seven
34+
distinct tests passed across six suites, with one pre-existing ignored case
35+
unchanged. The codegen suites validate the active generated-test path.
36+
Black 22.3.0 passed for 206 Python files; whitespace checks passed.
37+
- Documentation: the existing Maps removal comment now points to the offline
38+
coverage. Detailed retained/rejected candidates and source evidence belong in
39+
the PR description rather than a new contributor rule.
40+
41+
## Limits
42+
43+
No live Azure tests were run. There is no claim of measured pipeline wall-clock
44+
savings: the deleted runner was unused and the loop change removes two Spark
45+
test passes. No newly retired service suite was found to delete.
46+
47+
Official lifecycle references checked by the audit:
48+
49+
- [Bing Search retirement](https://learn.microsoft.com/en-us/lifecycle/announcements/bing-search-api-retirement).
50+
Those tests were already removed.
51+
- [Document Intelligence lifecycle table](https://learn.microsoft.com/en-us/azure/ai-services/document-intelligence/overview?view=doc-intel-4.0.0).
52+
The audit found v2.1 support ending September 15, 2027, not an already-retired endpoint.
53+
- [Anomaly Detector overview](https://learn.microsoft.com/en-us/azure/ai-services/anomaly-detector/overview).
54+
Its October 1, 2026 retirement is still future at the September 21 audit date;
55+
its tests had already been removed in the repository.
56+
57+
The Azure Maps announcement redirected to an application shell, so its text
58+
was not independently recovered. Existing source/history establishes why the
59+
old live test is absent, not a new deletion decision.
60+
61+
This is a direct review, not a completed six-round, three-family gauntlet.
62+
The advertised Gemini reviewers failed to launch with HTTP 400 during this
63+
task. GPT and Opus reviews were available for the larger companion CI change.
64+
Current-head Azure validation and GitHub review remain separate PR gates.

‎tools/pytest/run_all_tests.py‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

0 commit comments

Comments
 (0)