Skip to content

Commit d98671f

Browse files
Jamesclaude
authored andcommitted
Check in the analyze_sp fixtures so the suite is self-contained
analyzespTest read its four stored procedures from the parser library's SQL corpus over a relative path, which only resolved when gsp_java happened to be checked out beside this repository. 3384202 corrected that path, which fixed it locally and left the tests skipping on CI, since the corpus lives under a directory named private and is not published. The four scripts are now in src/test/resources/sqlscripts/analyze_sp/, copied byte for byte (cmp-verified against the corpus). That removes the sibling requirement rather than working around it: a fresh clone with no neighbour now runs 144 tests, 0 failures, 0 skipped. Verified by copying the tracked tree somewhere with no gsp_java sibling and running the full suite there. The test resolves them off the classpath rather than by relative path, so it no longer depends on the working directory either, and fails loudly with a named message if the resource is ever missing from the build. src/test/resources/sqlscripts/analyze_sp/readme.md records where they came from, what each one exercises, and that renaming anything inside them changes the relation strings the test compares against. sample7.sql producing no output is the assertion, not an oversight. check-test-results.sh keeps reporting skips and still fails when everything skipped, but nothing skips today; that guard is now for whatever gets added next. build.yml's comment no longer describes a skip that cannot happen. gspCommon keeps the corrected BASE_SQL_DIR for the commonTest/sqlenvTest classes that name it. None of those run: they are lowercase testXxx.java, which Surefire's default includes never match. Its javadoc now says to prefer a checked-in fixture, since a checked-in fixture cannot silently resolve to nothing, which is the failure this whole thread started from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qPRpoD8exYRrUmbfXXWXj
1 parent 3384202 commit d98671f

10 files changed

Lines changed: 405 additions & 66 deletions

File tree

.github/scripts/check-test-results.sh

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,10 @@
1010
# corrected the expected strings match the current parser's output exactly, and
1111
# the suite has no expected failures at all. Any failure is now a real one.
1212
#
13-
# Skips are reported rather than ignored. Tests that read the shared corpus skip
14-
# themselves when it is absent, which is the normal state on CI, since the
15-
# corpus lives in the gsp_java library repository under a directory named
16-
# private and is not published. A skip is therefore expected on CI and means
17-
# "not covered here", not "passed".
13+
# Skips are reported rather than ignored. Nothing skips today -- every input the
14+
# suite needs is checked in -- but a skip means "not covered here" rather than
15+
# "passed", so it is surfaced, and a run where EVERYTHING skipped fails outright
16+
# rather than reading as a pass.
1817
#
1918
# Usage: .github/scripts/check-test-results.sh [surefire-reports-dir]
2019
#
@@ -76,8 +75,7 @@ for name, kind, msg in sorted(bad):
7675
print("::error::%s: %s%s" % (kind, name, (" -- " + msg[0]) if msg else ""))
7776
7877
if skipped:
79-
print("::notice::%d test(s) skipped, needing the shared SQL corpus from the "
80-
"gsp_java repository (expected on CI):" % len(skipped))
78+
print("::notice::%d test(s) skipped -- skipped is not passed, check why:" % len(skipped))
8179
for name in sorted(skipped):
8280
print(" %s" % name)
8381

.github/workflows/build.yml

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,12 @@ jobs:
4444

4545
# This used to tolerate three failures in analyzespTest, believed to be
4646
# golden strings written for an older parser. They were something else:
47-
# gspCommon.BASE_SQL_DIR pointed one level short of the shared SQL corpus,
48-
# so the input file was never found and the comparison ran against an
49-
# empty string. With the path corrected the suite has no expected
50-
# failures, and `continue-on-error` is gone with them -- a red test run is
51-
# now a red build.
52-
#
53-
# Those four tests skip themselves on CI, where the corpus is absent: it
54-
# lives in the gsp_java repository under a directory named private and is
55-
# not published. check-test-results.sh reports what was skipped and fails
56-
# if everything was, so a run that proves nothing cannot read as a pass.
47+
# gspCommon.BASE_SQL_DIR pointed one level short of the library's SQL
48+
# corpus, so the input file was never found and the comparison ran against
49+
# an empty string. Those four scripts are now checked in under
50+
# src/test/resources/, so the suite is self-contained: 144 tests, no
51+
# expected failures, nothing skipped. `continue-on-error` went with the
52+
# threshold -- a red test run is now a red build.
5753
- name: Test
5854
run: mvn -B test
5955

README.md

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -184,36 +184,28 @@ directories carry their own `readme.md`.
184184
mvn test
185185
```
186186

187-
144 tests, all passing.
188-
189-
Four of them, in `gudusoft.gsqlparser.demosTest.analyzespTest`, read stored
190-
procedures from a SQL corpus that is **not in this repository**. It belongs to
191-
the parser library repo and is reached by a relative path, so it only resolves
192-
when the two sit side by side:
193-
194-
```
195-
github/
196-
gsp_java/ <- library, owns gsp_java_core/gsp_sqlfiles/
197-
gsp_demo_java/ <- this repository
198-
```
199-
200-
Without that neighbour those four **skip**, and `mvn test` still passes. The
201-
corpus is under a directory named `private` and is not published, so skipping
202-
is the normal state on CI and in a plain clone. A skip means "not covered
203-
here", not "passed", which is why `check-test-results.sh` lists what was
204-
skipped and fails if *everything* was.
205-
206-
> **Correction.** Until 2026-07-28 this section said three of those tests
207-
> failed because their expected output was written for an older parser and had
208-
> since drifted. That was wrong, and it sent people looking at parser output
209-
> for a bug that was never there. `gspCommon.BASE_SQL_DIR` read
210-
> `../gsp_java_core/`, one level short: it named a sibling of this checkout
211-
> rather than the module inside `gsp_java`, so it resolved to nothing at all.
212-
> `Analyze_SP` never found an input file, returned an empty string, and the
213-
> comparison failed. The expected strings match the current parser's output
214-
> exactly, character for character, on all three. `testSample7` "passed"
187+
144 tests, all passing, nothing skipped. Every input the suite needs is in the
188+
repository, so a plain clone runs the whole thing.
189+
190+
> **Correction.** Until 2026-07-28 this section said three tests in
191+
> `gudusoft.gsqlparser.demosTest.analyzespTest` failed because their expected
192+
> output was written for an older parser and had since drifted, and that they
193+
> were kept red on purpose as a drift signal. That was wrong, and it sent
194+
> people looking at parser output for a bug that was never there.
195+
>
196+
> Those tests read stored procedures from the library's SQL corpus over a
197+
> relative path, `gspCommon.BASE_SQL_DIR`. The path read `../gsp_java_core/`,
198+
> one directory level short: it named a sibling of this checkout rather than
199+
> the module inside `gsp_java`, so it resolved to nothing at all. `Analyze_SP`
200+
> never found an input file, returned an empty string, and comparing that with
201+
> the expected output failed. The expected strings match the current parser's
202+
> output exactly, character for character, on all three. `testSample7` "passed"
215203
> throughout only because it expects empty output, which is also what a missing
216-
> file produces. Fixing the path turned 3 failures into 0.
204+
> file produces, so it was asserting nothing.
205+
>
206+
> The four scripts now live in `src/test/resources/sqlscripts/analyze_sp/`,
207+
> copied byte for byte from the corpus, which removes the sibling-checkout
208+
> requirement rather than just correcting it. See that directory's `readme.md`.
217209
218210
Every test here exercises a **demo** in this repository. The tests that
219211
exercise the *parser* live in the library, not here — see "The library-side
@@ -554,9 +546,10 @@ Three scripts under `.github/scripts/`, all runnable locally:
554546
- **`check-test-results.sh`** — parses the surefire XML and fails on any
555547
failure or error. There are no expected failures: the three that used to be
556548
tolerated were a wrong fixture path, not parser drift (see "Running the
557-
tests"). It also lists what was skipped, and fails if *every* test was — a
549+
tests"). It also lists anything skipped, and fails if *every* test was — a
558550
run that proved nothing should not read as a pass just because no assertion
559-
got far enough to fail.
551+
got far enough to fail. Nothing skips today; that guard is there for whatever
552+
gets added next.
560553
- **`run-all-demos.sh`** — launches **every** class with a `main()`, taken from
561554
the compiled output so anything `pom.xml` excludes is excluded here too. It
562555
checks that each demo *starts*: `NoClassDefFoundError`, `NoSuchMethodError`

src/test/java/gudusoft/gsqlparser/commonTest/gspCommon.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,17 @@
2626
* match the current parser's output exactly, character for character.
2727
* Corrected on 2026-07-28.
2828
*
29-
* <p>Tests reading from here must call {@link #sqlFilesAvailable()} and skip
30-
* themselves when it returns false. The corpus sits under a directory named
31-
* {@code private} and is not published, so it is absent on CI and in any clone
32-
* that does not also have the library checked out beside it.
29+
* <p>{@code analyzespTest} no longer reads from here at all. Its four scripts
30+
* were copied into {@code src/test/resources/sqlscripts/analyze_sp/}, so it runs
31+
* from a plain clone with no sibling repository. **Prefer that for anything
32+
* new**: a checked-in fixture cannot silently resolve to nothing.
33+
*
34+
* <p>What still names these constants is the {@code commonTest} and
35+
* {@code sqlenvTest} classes, none of which run: they are named lowercase
36+
* {@code testXxx.java}, which Surefire's default includes never match. If you
37+
* ever enable one, call {@link #sqlFilesAvailable()} and skip when it returns
38+
* false, because the corpus sits under a directory named {@code private}, is not
39+
* published, and is absent on CI and in any clone without the library beside it.
3340
*/
3441
public class gspCommon {
3542
public static String BASE_SQL_DIR = "../gsp_java/gsp_java_core/gsp_sqlfiles/TestCases/";

src/test/java/gudusoft/gsqlparser/demosTest/analyzespTest.java

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,40 +2,50 @@
22
package gudusoft.gsqlparser.demosTest;
33

44
import java.io.File;
5+
import java.net.URL;
56
import java.util.ArrayList;
67
import java.util.List;
78

8-
import gudusoft.gsqlparser.commonTest.gspCommon;
99
import gudusoft.gsqlparser.demos.analyzesp.Analyze_SP;
1010

11-
import org.junit.Assume;
1211
import org.junit.Before;
1312
import org.junit.Test;
1413

14+
import static org.junit.Assert.assertNotNull;
1515
import static org.junit.Assert.assertTrue;
1616

1717
/**
18-
* These read their input from the shared SQL corpus in the gsp_java library
19-
* repository, which is absent unless that repository is checked out beside this
20-
* one, so they skip rather than fail when it is missing. See
21-
* {@link gspCommon} for why, and for the path bug that made them look like
22-
* output drift for as long as they did.
18+
* Input comes from {@code src/test/resources/sqlscripts/analyze_sp/}, so these
19+
* run anywhere the repository is checked out, CI included.
2320
*
24-
* <p>JUnit 4 annotations rather than extending TestCase, deliberately:
25-
* Assume inside a JUnit 3 TestCase is reported by surefire as an ERROR, not as
26-
* a skip, so the skip would have been indistinguishable from the failure it
27-
* replaces.
21+
* <p>They used to read the shared SQL corpus in the gsp_java library repository
22+
* over a relative path, which only resolved when the two repositories sat side
23+
* by side. That path was also one directory level short, so it resolved
24+
* nowhere at all: no input file was found, {@code Analyze_SP} returned an empty
25+
* string, and comparing that with the expected output failed. Those three
26+
* failures were written up for a long time as the parser's output drifting away
27+
* from stale golden strings, which they never were. The four scripts are now
28+
* checked in beside the test, so there is no sibling to get wrong. See
29+
* {@code src/test/resources/sqlscripts/analyze_sp/readme.md} for where they
30+
* came from.
31+
*
32+
* <p>JUnit 4 annotations rather than extending {@code TestCase}: kept from when
33+
* these skipped on a missing corpus, since {@code Assume} inside a JUnit 3
34+
* {@code TestCase} is reported by surefire 2.12.4 as an error rather than a
35+
* skip.
2836
*/
2937
public class analyzespTest
3038
{
3139

3240
private String basedir;
3341

3442
@Before
35-
public void setUp( )
43+
public void setUp( ) throws Exception
3644
{
37-
Assume.assumeTrue( gspCommon.whySqlFilesMissing( ), gspCommon.sqlFilesAvailable( ) );
38-
basedir = gspCommon.BASE_SQL_DIR+"private/sqlscripts/analyze_sp";
45+
URL dir = analyzespTest.class.getResource( "/sqlscripts/analyze_sp" );
46+
assertNotNull( "test fixtures are missing from src/test/resources/sqlscripts/analyze_sp",
47+
dir );
48+
basedir = new File( dir.toURI( ) ).getPath( );
3949
}
4050

4151
@Test
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# analyze_sp fixtures
2+
3+
The four SQL Server scripts here are the input for
4+
`gudusoft.gsqlparser.demosTest.analyzespTest`, which exercises the `analyzesp`
5+
demo (`Analyze_SP`) and its stored-procedure relation output.
6+
7+
They are copied verbatim, byte for byte, from the parser library's shared test
8+
corpus:
9+
10+
```
11+
gsp_java/gsp_java_core/gsp_sqlfiles/TestCases/private/sqlscripts/analyze_sp/
12+
```
13+
14+
| file | exercises |
15+
|---|---|
16+
| `sample1.sql` | temp tables, cursors, `Create` / `Insert` / `Update` / `Read` / `Drop` relations |
17+
| `sample6.sql` | one table written three ways, plus a `Delete` |
18+
| `sample7.sql` | dynamic SQL that yields no static relations at all |
19+
| `sample8.sql` | joins across four tables feeding a temp table |
20+
21+
## Why they are checked in here
22+
23+
They used to be read from that corpus over a relative path, which only resolved
24+
when `gsp_java` happened to be checked out beside this repository. It also had
25+
the path one directory level short, so it resolved nowhere: `Analyze_SP` never
26+
found an input file, returned an empty string, and the comparison against the
27+
expected output failed. Three of these tests were red for a long time and were
28+
documented as the parser's output having drifted from stale golden strings.
29+
They had not drifted. The expected strings match the current parser's output
30+
exactly, character for character.
31+
32+
Checking the scripts in removes the sibling-checkout requirement entirely, so
33+
the tests run on CI and in a plain clone.
34+
35+
## Editing them
36+
37+
The expected output in `analyzespTest` is matched exactly. Changing a table,
38+
column or procedure name here changes the relation strings the test compares
39+
against, so regenerate those in the same commit. `sample7.sql` is expected to
40+
produce **no** output; that is the assertion, not an oversight.
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
USE [Retail]
2+
GO
3+
/****** Object: StoredProcedure [dbo].[usp_CreateUrunIcerikForKoliKabul] Script Date: 05/30/2012 09:56:07 ******/
4+
SET ANSI_NULLS ON
5+
GO
6+
SET QUOTED_IDENTIFIER ON
7+
GO
8+
ALTER proc [dbo].[usp_CreateUrunIcerikForKoliKabul] (@ProcessID bigint)
9+
10+
as
11+
12+
begin
13+
14+
Create table #tmpKoliKabulCheck
15+
(
16+
Barkod nvarchar(50),
17+
IrsaliyeNo bigint,
18+
GonderenDepo nvarchar(50),
19+
AlanDepo nvarchar(50),
20+
OkutmaTarihi nvarchar(50),
21+
OkutanKullanici nvarchar(100)
22+
)
23+
24+
insert into #tmpKoliKabulCheck
25+
exec usp_TemaMobileParseDataForKoliKabulCheck @ProcessID, 3
26+
27+
update #tmpKoliKabulCheck set Barkod = replace(Barkod, '#', '_')
28+
29+
select *,
30+
(select top 1 Data from dbo.fnk_Split(KoliBarkod, '_')) ToplamaId,
31+
(select top 1 Data from dbo.fnk_Split(KoliBarkod, '_') order by ID desc) ToplamaKoliId
32+
into #tmpCurs
33+
from (
34+
select UrunBarkod, KoliBarkod,
35+
COUNT(UrunBarkod) as FiiliMiktar, 0 KaydiMiktar from (
36+
SELECT TranData.value('(Barkod/@parentKey)[1]',
37+
'varchar(max)') AS KoliBarkod,
38+
TranData.value('(Barkod/@value)[1]',
39+
'varchar(max)') AS UrunBarkod
40+
FROM tb_TemaMobileCommandTransaction where TemaMobileTransactionTipTanimRef = 1 and IsDeleted = 0
41+
) k
42+
group by UrunBarkod, KoliBarkod
43+
) x
44+
where KoliBarkod in
45+
(select Barkod from #tmpKoliKabulCheck)
46+
47+
48+
49+
DECLARE kolicursor Cursor
50+
FOR
51+
Select ToplamaId, ToplamaKoliId, UrunBarkod from #tmpCurs
52+
open kolicursor
53+
54+
declare @m_ToplamaId int
55+
declare @m_ToplamaKoliId int
56+
declare @m_UrunBarkod nvarchar(50)
57+
58+
fetch next from kolicursor into @m_ToplamaId, @m_ToplamaKoliId, @m_UrunBarkod
59+
While (@@FETCH_STATUS <> -1)
60+
BEGIN
61+
IF (@@FETCH_STATUS <> -2)
62+
begin
63+
64+
DECLARE @Output VARCHAR(100)
65+
66+
CREATE TABLE #tmpTable
67+
(
68+
OutputValue VARCHAR(100)
69+
)
70+
INSERT INTO #tmpTable (OutputValue)
71+
EXEC usp_GetKoliInfoByBarkod @m_ToplamaId, @m_ToplamaKoliId, @m_UrunBarkod
72+
73+
if exists (select Top 1 1 from #tmpTable)
74+
begin
75+
SELECT
76+
@Output = OutputValue
77+
FROM
78+
#tmpTable
79+
end
80+
else
81+
begin
82+
set @Output = 0
83+
end
84+
85+
86+
DROP TABLE #tmpTable
87+
88+
update #tmpCurs set KaydiMiktar = @Output
89+
where UrunBarkod = @m_UrunBarkod and ToplamaId = @m_ToplamaId and ToplamaKoliId = @m_ToplamaKoliId
90+
end
91+
92+
FETCH NEXT FROM kolicursor INTO @m_ToplamaId, @m_ToplamaKoliId, @m_UrunBarkod
93+
END
94+
CLOSE kolicursor
95+
DEALLOCATE kolicursor
96+
97+
select * from #tmpCurs
98+
99+
end
100+
101+

0 commit comments

Comments
 (0)