Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add PharoXX-13 image selection #643

Merged
merged 1 commit into from
May 26, 2024
Merged

Conversation

theseion
Copy link
Collaborator

No description provided.

Copy link
Member

@fniephaus fniephaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@theseion theseion force-pushed the add-pharo-13 branch 2 times, most recently from b5934fc to 1dbe31b Compare May 21, 2024 19:09
@theseion
Copy link
Collaborator Author

@dalehenrich GemStone tests are failing, #testSentButNotImplemented. I suppose it's the use of #at:ifAbsent: that I introduced. Any idea how to handle this?

Copy link
Collaborator

@estebanlm estebanlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine, thanks :)
and as I said... if this is staled is a problem on Pharo-13 that will also be there in Pharo-alpha so... we need to work on making it not-stale :P

@fniephaus
Copy link
Member

I suppose it's the use of #at:ifAbsent: that I introduced. Any idea how to handle this?

Avoid the use of #at:ifAbsent: and use #at: and #iNil: instead?

@dalehenrich
Copy link
Collaborator

Sorry, just noticed this. According to the log, it looks like there is a method #skip introduced introduced in SCIGoferLoadSpecTest>>testAddLoadedClassesFrom ... found this by looking in the log for the string Sent but not implemented:

2024-05-21T19:13:30.0518150Z .'Sent but not implemented from #''SCIGoferLoadSpecTest>>testAddLoadedClassesFrom'''
2024-05-21T19:13:30.0519082Z 	'#''skip'''

So it isn't at:ifAbsent: that's the issue ... presumably this is in code that is not used by GemStone? #perform: could be used ...

Here's the source and it seems that #skip is expected to be implemented in a superclass or ???

testing
testAddLoadedClassesFrom
	| goferClass gofer |
	goferClass := Smalltalk
		at: #Gofer
		ifAbsent: [ ^ self skip ].
	
	self assert: true equals: (self spec loadedClasses isEmpty).
	
	gofer := goferClass new
		package: 'SmalltalkCI-Core';
		package: 'SmalltalkCI-Tests'.
	self spec addLoadedClassesFrom: gofer references.
	self deny: (self spec loadedClasses isEmpty).
	self assert: (self spec loadedClasses includes: SmalltalkCI).
	self assert: (self spec loadedClasses includes: SmalltalkCITest).

... hmmm, Gofer is present in GemStone, so the test must pass for GemStone ...

@theseion
Copy link
Collaborator Author

Thanks @dalehenrich. I added the #skip to skip the test for Pharo 13, since it no longer has Gofer. Is there a similar message I could send for GemStone to skip the test?

Avoid the use of #at:ifAbsent: and use #at: and #iNil: instead?

Yes, I tried that, but SystemDictionary in Pharo 13 apparently no longer understands #at:.

@dalehenrich
Copy link
Collaborator

dalehenrich commented May 24, 2024

@theseion

Thanks @dalehenrich. I added the #skip to skip the test for Pharo 13, since it no longer has Gofer. Is there a similar message I could send for GemStone to skip the test?

No ... well, an early ^self will "skip the test" ... I assume that Pharo sunit must keep track of skipped tests?

@theseion
Copy link
Collaborator Author

I see. Since this test is internal, I'll just use ^ self.

@theseion theseion merged commit cdd3710 into hpi-swa:master May 26, 2024
78 checks passed
@theseion theseion deleted the add-pharo-13 branch May 26, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants