Skip to content

Conversation

@dionisiydk
Copy link
Contributor

@dionisiydk dionisiydk commented Aug 25, 2021

The test #testIsExecutionFinished is cleaned to not depend on the particular implementation of Process>>#terminate method. And this clean version actually shows that the debugger does not work correctly. Therefore the test is disabled on CI for now.

This change is required to properly integrate #realActiveProcess PR (pharo-project/pharo#8567) into Pharo without breaking the existing tests of Sindarin project.
Once Pharo PR will be integrated the fix for Sindarin test will be pushed. The SindarinDebugger methods are required for the this fix but they do not break other parts in current image and therefore they are included here.

IMPORTANT: integrate it into Pharo10 branch or do a proper release version. It seems we lost the static versions with this project. Any update to master here will affect all Pharo builds.

…ticular implementation of Process>>#terminate method. And this clean version actually shows that the debugger does not work correctly. Therefore the test is disabled on CI for now.

This change is required to properly integrate PR#8567 (realActiveProcess) into Pharo without breaking the existing tests of Sindarin project.
Once Pharo PR  will be integrated the fix for Sindarin test will be pushed. The SindarinDebugger methods are required for the this fix but they do not break other parts in current image and therefore they are included here.
@dionisiydk
Copy link
Contributor Author

Hi guys.
I see you just approved it. But I believe it should go into separate branch Pharo10 to not affect new Pharo9 builds (the baseline simply uses master).
I can't create another branch from my side.

@StevenCostiou StevenCostiou changed the base branch from master to Pharo-10 September 3, 2021 10:04
@StevenCostiou
Copy link
Member

Not sure it is what you meant:

  • I created a P10 branch from master
  • I changed the branch to merge this PR to that P10 branch
    But how will the P10 and master later be synchronized?

@dionisiydk
Copy link
Contributor Author

Not sure it is what you meant:

  • I created a P10 branch from master
  • I changed the branch to merge this PR to that P10 branch
    But how will the P10 and master later be synchronized?

It is wrong to use "raw" master branch in the entire Pharo baseline for given version:

  • Pharo9 release depends on Sindarin master branch.
  • Any update to Sindarin will silently change the Pharo9 codebase: any new builds will contain new version of Sindarin and nobody will notice it

The right approach would be to have a release tag which Pharo9 can depend on statically.
Now we can't update the master until it will be fixed in Pharo9.1 .

But my suggestion for Pharo10 branch here is based on the way how other dependencies are now managed. You can notice Pharo10 branch in Spec2 and NewTools projects for example.

@estebanlm please correct me if I am wrong.

@dionisiydk
Copy link
Contributor Author

And could you guys merge it now?
It will anyway have no effect until it will be integrated into Pharo10.

@dionisiydk
Copy link
Contributor Author

@StevenCostiou does anything prevent a merge?

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.

3 participants