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

Backport fixed tests #14409

Merged
merged 2 commits into from
Aug 2, 2023
Merged

Conversation

PalumboN
Copy link
Collaborator

@PalumboN PalumboN commented Aug 2, 2023

Backport a fixed test + marking two tests as longTestCase.

This is for having the VM CI in GREEN 🟢

See pharo-project/pharo-vm#661

jecisc and others added 2 commits August 2, 2023 13:34
One coverage tests has been disabled after failing randomly on the CI. The problems comes from some VM optimizations on the VM.
In linux, special selectors  are running in the interpreter so the messages Point >> #x and Point >> #y are never executed.
In OSX, the method is compiled to machine code and the messages are sent.

To have a more robust test, I excluded the special selectors from the list of methods to cover
Copy link
Member

@guillep guillep left a comment

Choose a reason for hiding this comment

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

LGTM

coverage := collector runOn: [ (1@1 corner: 2@2) center ]. "Setup, execute and teardown."
self assert: (coverage methods includes: Point>>#x). "Inspect the results"
self assert: (coverage nodes size > 10). "Covered paths are also available"
"We skip the method with special selectors because them might not be detected by the coverage collector in case they are optimized by the VM."
Copy link
Member

Choose a reason for hiding this comment

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

ok!

@Ducasse Ducasse merged commit 4670a55 into pharo-project:Pharo11 Aug 2, 2023
1 check passed
@PalumboN PalumboN deleted the backport-fixed-tests branch August 2, 2023 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants