hpi-swa / smalltalkCI

Framework for testing Smalltalk projects with GitHub Actions, GitLab CI, Travis CI, AppVeyor, and others.
MIT License
94 stars 68 forks source link

KeyNotFound: key #Author not found in SystemEnvironment #655

Closed hernanmd closed 3 weeks ago

hernanmd commented 1 month ago

Pharo CI builds are failing (for instance https://github.com/pharo-spec/Spec/actions/runs/10426671395/job/28880047921?pr=1587) because SmalltalkCI is using Author that now (Pharo 13) is removed from the image:

MetacelloNotification: Loaded -> BaselineOfSmalltalkCI --- unresolved
MetacelloNotification: Loading baseline of BaselineOfSmalltalkCI...
NewUndeclaredWarning: SCIGoferLoadSpec>>basicLoadProjectOn: (Gofer is Undeclared)
NewUndeclaredWarning: SCIMetacelloLoadSpec>>basicLoadProjectOn: (MetacelloIgnorePackageLoaded is Undeclared)
MetacelloNotification: Loaded -> SmalltalkCI-Core --- filetree:///home/runner/.smalltalkCI/repository [:]()
NewUndeclaredWarning: SmalltalkCIPharo9>>imageInfo (StSystemReporter is Undeclared)
MetacelloNotification: Loaded -> SmalltalkCI-Pharo-Core --- filetree:///home/runner/.smalltalkCI/repository [:]()
MetacelloNotification: Loaded -> SmalltalkCI-Coverage-Core --- filetree:///home/runner/.smalltalkCI/repository [:]()
MetacelloNotification: Loaded -> SmalltalkCI-Pharo-Coverage-Core --- filetree:///home/runner/.smalltalkCI/repository [:]()
MetacelloNotification: ...finished baseline
KeyNotFound: key #Author not found in SystemEnvironment
SystemEnvironment(Dictionary)>>errorKeyNotFound:
[self errorKeyNotFound: key] in SystemEnvironment(Dictionary)>>at: in Block: [self errorKeyNotFound: key]
SystemEnvironment(Dictionary)>>at:ifAbsent:
SystemEnvironment(Dictionary)>>at:
SmalltalkImage>>at:
SmalltalkCIPharo13 class(SmalltalkCIPharo class)>>getAuthor
SmalltalkCIPharo13 class(SmalltalkCI class)>>ensureAuthor
SmalltalkCIPharo13(SmalltalkCI)>>initialize
SmalltalkCI class>>newInstance
SmalltalkCI class>>newInstanceFor:projectDirectory:
SmalltalkCI class>>load:projectDirectory:
SmalltalkCI class>>load:
UndefinedObject>>DoIt
OCReceiverDoItSemanticScope(OCDoItSemanticScope)>>evaluateDoIt:
OpalCompiler>>evaluate
OpalCompiler>>evaluate:
fniephaus commented 1 month ago

@theseion could you take a look again, please? Is Pharo not gracefully phasing out API or why keep they breaking smalltalkCI every now and then?

guillep commented 1 month ago

Hi @fniephaus ! How are you?

I've not followed in detail all the incompatibilities you mention. As for this case, Author was deprecated in Pharo12, about one year ago, and just got removed now in the alpha of Pharo13. I'd expect Pharo13 (which is the alpha version) to be a bit more shaky.

Of course, if some incompatibility is introduced without a proper deprecation I believe the right way to do it is to revert and deprecate on the Pharo side, no change to be done on smalltalkci.

Would you like that we manage smalltalkci integration a bit differently? Besides a good management of deprecations, maybe we could reduce the noise of these issues?

fniephaus commented 4 weeks ago

Hi @guillep, I'm good, hope you are too. I think it's fine if API is changed and sometimes phased out. What I'd like to see is that smalltalkCI gets updated when API is deprecated, not when it's removed. Is the deprecation warning swallowed somehow by smalltalkCI or why has no one complained that smalltalkCI is still using Author in the last year? :)

hernanmd commented 4 weeks ago

Hi guys, I want to fix this issue. But I need some information. Could you tell me how you load the Baseline in Pharo 13 so SmalltalkCIPharo13 is included?

I loaded the Baseline as follows in Pharo 13:

Metacello new
    baseline: 'SmalltalkCI';
    repository: 'github://hernanmd/smalltalkCI/repository';
    load.

but SmalltalkCIPharo13 is not found anywhere.

Rinzwind commented 4 weeks ago

@hernanmd You’re loading from your fork, which is currently several commits behind (see screenshot below) and doesn’t include the class SmalltalkCIPharo13 (added in commit 3b1a8b0b7bbd6b9d), you should sync it first.

hernanmd commented 4 weeks ago

@hernanmd You’re loading from your fork, which is currently several commits behind (see screenshot below) and doesn’t include the class SmalltalkCIPharo13 (added in commit 3b1a8b0), you should sync it first.

Thanks, I missed that one!

theseion commented 3 weeks ago

In this particular case, using Author never produced any deprecation warnings. There is only a meta method Author class>>isDeprecated that returns true. So we would never have been able to detect this deprecation on the CI.

In case of other deprecations, we usually only catch them when the alpha starts throwing them out. This is because we simply don't see them. The log might show them, but they don't lead to test failures or produce Smalltalk CI specific output. For Pharo specifically it would be easy to handle by wrapping the test code in Deprecation class>>deprecationsWhile:, which answers a set of encountered deprecations.

Personally, I think we could do the following:

This way, normal PRs would not be impacted by new deprecations and we wouldn't be surprised by sudden failures in the alpha (there will still be times where breaking changes in the alpha will be unavoidable).

theseion commented 3 weeks ago

@guillep, the idea outlined above will not help us with class deprecations. I feel that there should be a mechanism on the side of Pharo to be warned about these. For SmalltalkCI, we don't care about deprecations if they don't impact us, so I wouldn't want to search the image for overrides of #isDeprecated.