Seaside v3.4.5 does not load tests correctly for 3.6.2 (unreleased GemStone version)

[dalehenrich]

It looks like the dependencies for Seaside-Tests-Canvas are broken as the package Seaside-Tests-Canvas is getting loaded BEFORE Seaside-Component:

--transcript--'Loaded -> Javascript-Tests-Core-cypress.1 --- github://SeasideSt/Seaside:master/repository [4205b38:master] --- cache'
--transcript--'Warning: This package depends on the following classes:
  WAComponent
  WADecoration
You must resolve these dependencies before you will be able to load these definitions: 
  WACanvasTestComponent
  WACanvasTestComponent>>renderContentOn:
  WACanvasTestDecoration
  WACanvasTestDecoration classSide>>rendererClass
  WACanvasTestDecoration classSide>>tagName:
  WACanvasTestDecoration>>initialize
  WACanvasTestDecoration>>renderContentOn:
  WACanvasTestDecoration>>tagName
  WACanvasTestDecoration>>tagName:
'
--transcript--'Loaded -> Seaside-Tests-Canvas-cypress.1 --- github://SeasideSt/Seaside:master/repository [4205b38:master] --- cache'
--transcript--'Loaded -> Seaside-Session-cypress.1 --- github://SeasideSt/Seaside:master/repository [4205b38:master] --- cache'
--transcript--'Loaded -> Seaside-GemStone-Session-cypress.1 --- github://SeasideSt/Seaside:master/repository [4205b38:master] --- cache'
--transcript--'Loaded -> Seaside-Tests-Session-cypress.1 --- github://SeasideSt/Seaside:master/repository [4205b38:master] --- cache'
--transcript--'Loaded -> Seaside-Component-cypress.1 --- github://SeasideSt/Seaside:master/repository [4205b38:master] --- cache'

I looked at the CI actions for v3.4.5 and it looks like the test for GemStone 3.6.1 (a released GemStone version) didn't actually finish loading tODE, let alone Seaside so it's hard for me to tell it ever passed ... Since the actions have been consistently failing for the last two months it's hard to tell when this particular bug was introduced ... So using github actions to replace Travis-CI (I know we have no choice) is not working out very well ... if we must ignore failures on a major release)... Yikes the logs expire after a certain amount of time ... 5 months ago the 3.6.0 action failed at the merge, but without logs, I can't tell what the failure was (I would only know that my local tests were passing otherwise I wouldn't have submitted the pull request that just added 3.7, 3.8, 3.9 to the baseline) ... so maybe this bug has been around since ???

Anyway, my 3.6.2 build doesn't fail due to the above Warning), but the WABuilderCanvasTest >> #testBodyWithDecorations test fails with an MNU for #tagName: (not loaded because of the dependency error), so the missing classes never made it into the image ...

I did look at the passing GemStone 3.5.5 test of v3.4.5 and scanned the raw log for Seaside-Tests-Canvas and while the order of the two packages is the same Seaside-Tests-Canvas BEFORE Seaside-Component, we did not get the same warning (and of course the tests passed):

2021-08-18T06:54:58.2060808Z --transcript--'Loaded -> Javascript-Tests-Core-cypress.1 --- filetree:///home/runner/work/Seaside/Seaside/repository [d717b748:HEAD] --- cache'
2021-08-18T06:54:58.5391723Z --transcript--'Loaded -> Seaside-Tests-Canvas-cypress.1 --- filetree:///home/runner/work/Seaside/Seaside/repository [d717b748:HEAD] --- cache'
2021-08-18T06:54:58.8977068Z --transcript--'Loaded -> Seaside-Session-cypress.1 --- filetree:///home/runner/work/Seaside/Seaside/repository [d717b748:HEAD] --- cache'
2021-08-18T06:54:58.9988349Z --transcript--'Loaded -> Seaside-GemStone-Session-cypress.1 --- filetree:///home/runner/work/Seaside/Seaside/repository [d717b748:HEAD] --- cache'
2021-08-18T06:54:59.3089489Z --transcript--'Loaded -> Seaside-Tests-Session-cypress.1 --- filetree:///home/runner/work/Seaside/Seaside/repository [d717b748:HEAD] --- cache'
2021-08-18T06:54:59.6556722Z --transcript--'Loaded -> Seaside-Component-cypress.1 --- filetree:///home/runner/work/Seaside/Seaside/repository [d717b748:HEAD] --- cache'

I wouldn't expect different Metacello behavior for different versions of GemStone but that appears to be the case as the baseline doesn't differentiate between 3.5.x and 3.6.x ...

Is Seaside still using #linear load? If we could use #atomic load, then package load order would be a moot point.

I will start spelunking around and try to characterize the issue. I guess the first order of business is to see when if ever the 3.6.0 tests were passing ...

[dalehenrich]

Our last passing internal test was run on Aug 16 at 3am PDT, which is 2 days before d717b7485 (which was tagged as the 3.4.5 release) so the regression appears to be fairly recent ... I'll run an internal test with 17b43cadb to confirm and then go hunting ...

[jbrichau]

There is one test that fails regularly for 3.6.1 but I have seen it passing as well. For now, I was thinking about a test timing issue I need to look at, but not a real problem.

So, where do you see it not running tests? E.g https://github.com/SeasideSt/Seaside/runs/3117498606?check_suite_focus=true shows the one failing test.

Am I overlooking something?

[dalehenrich]

I ran a build with 17b43ca and it seems that the classes WACanvasTestComponent and WACanvasTestDecoration aren't present and the test case WABuilderCanvasTest >> #testBodyWithDecorations isn't present either. So when the new classes and test case was added, it was either a mis-packaging of the two classes or a missing required package.

Perhaps the following in BaselineOfSeaside3>>baselinecommon: would fix the error?

  package: 'Seaside-Tests-Canvas'
    with: [ spec requires: #('Seaside-Tests-Core' 'Seaside-Canvas' 'Seaside-Component') ];

Surprising that this doesn't happen in Pharo, but it's possible that the load ordered is affected by something in BaselineOfSeaside3>>baselinepharo:?

@jbrichau, I'll leave the resolution to you ... we'll resume running the internal tests once the fix has made it into onto the head of the master branch ...

[dalehenrich]

So, where do you see it not running tests? It was the 3.6.1 test run when v3.4.5 was tagged that failed during load of tODE and if run should have shown the test failure I was seeing.

I agree with you that the test run you referred to looks like it could be a timing problem (i.e., heavily loaded machine) ...

[jbrichau]

The load dependency got introduced in #1252 and went undetected... which is not good because the test testBodyWithDecorations is not run in any of the builds as a result.

Instead of adding a dependency from Seaside-Canvas to Seaside-Component, I decided to move the tests (and part of the implementation) to Seaside-RenderLoop. Since the use of the WABuilder in the context of Seaside components requires both the Canvas and Components package, it seems a more natural fit for the RenderLoop package.

[jbrichau]

@dalehenrich thanks for reporting this. I created a new issue #1280 to deal with the fact this went unnoticed in the CI builds. The changes I committed in https://github.com/SeasideSt/Seaside/commit/9605f4fce98968ff3a6eab53923e42e58964d878 should put the code in a more appropriate place that respects the package dependencies.

Let me know if this fixes the issue.

[dalehenrich]

@jbrichau thanks for the quick response! I ran a manual load (that was "failing" before) and the new test class WABuilderWithComponentsCanvasTest is cleanly loaded and the test passes ...

[jbrichau]

@dalehenrich I am correct that this one can be closed? If not, please shout ;-)