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

adding traces in SpMorphicMillerAdapter to trace problems in CI #665

Conversation

adri09070
Copy link
Contributor

Trying to fix #664

@adri09070 adri09070 added the need more work This needs to be improved before being considered label Jan 22, 2024
@adri09070 adri09070 closed this Jan 22, 2024
@adri09070 adri09070 reopened this Jan 22, 2024
@adri09070 adri09070 closed this Jan 22, 2024
@adri09070 adri09070 reopened this Jan 22, 2024
@adri09070 adri09070 closed this Jan 22, 2024
@adri09070 adri09070 reopened this Jan 22, 2024
@Ducasse
Copy link
Contributor

Ducasse commented Jan 22, 2024

do you need it in Pharo or just your PR is enough to get the trace?

@adri09070
Copy link
Contributor Author

adri09070 commented Jan 22, 2024

This is just for me to investigate so I think the PR should be enough (and if it should be in Pharo, I should do a PR in pharo-spec/Spec instead ).
I discovered what causes problem here but I don't think this is a debugger problem. This is a problem in Morphic/SpecMorph adapters.

In SpMorphicMillerAdapter>>#scrollToShowLastPage, a bloc evaluation is defered which show a submorph of its inner widget. However, sometimes its inner widget has no submorph and the adapter tries to show the morph at index 0, which causes this random error on the CI.
I don't think this is something I can control in the debugger. Maybe @estebanlm has an idea as to how the SpMorphicMillerAdapter's inner widget happens to have no submorph ?

@adri09070
Copy link
Contributor Author

adri09070 commented Jan 22, 2024

Example of CI run that went wrong with traces: https://github.com/pharo-spec/NewTools/actions/runs/7611736592/job/20727806873?pr=665

Example of CI run that went smoothly with traces: https://github.com/pharo-spec/NewTools/actions/runs/7611658534/job/20727565575

Also another example of CI run that went wrong and in which I don't know what the hell happened because there are completely different errors with identical (but less numerous) traces: https://github.com/pharo-spec/NewTools/actions/runs/7611490325/job/20727060848

@adri09070
Copy link
Contributor Author

An ugly solution that could at least fix the CI is to add an innerWidget submorphs ifNotEmpty: guard but I'm pretty sure someone familiar with Morphic would be better suited to investigate and fix it

@Ducasse
Copy link
Contributor

Ducasse commented Jan 22, 2024

Let us fix it in a ugly way because there is no Morphic expert and we are working for killing morphic :)

@estebanlm
Copy link
Member

the problem seems to be a race condition, not directly a morphic problem, heh.
in anycase, I will merge if you promise to revert as soon as you have the information you need :)

@estebanlm
Copy link
Member

An ugly solution that could at least fix the CI is to add an innerWidget submorphs ifNotEmpty: guard but I'm pretty sure someone familiar with Morphic would be better suited to investigate and fix it

you have Morph>>#hasSubmorphs method, no need to break demeters law.

@estebanlm
Copy link
Member

btw, you know you can run smalltalkci locally, isn't?
that may help you to find the problem without charging the real CI

@adri09070
Copy link
Contributor Author

btw, you know you can run smalltalkci locally, isn't? that may help you to find the problem without charging the real CI

How do you do that?

@estebanlm
Copy link
Member

btw, you know you can run smalltalkci locally, isn't? that may help you to find the problem without charging the real CI

How do you do that?

you checkout https://github.com/hpi-swa/smalltalkCI and from terminal execute something like:

./bin/smalltalkci -s Pharo64-12 /home/esteban/dev/repo/pharo-vcs/iceberg/.smalltalk.ston

@adri09070
Copy link
Contributor Author

I think I fixed this error, but I now have a different (but similar) one (that was already there: https://github.com/pharo-spec/NewTools/actions/runs/7603649968/job/20705636397?pr=660)

@adri09070
Copy link
Contributor Author

btw, you know you can run smalltalkci locally, isn't? that may help you to find the problem without charging the real CI

How do you do that?

you checkout https://github.com/hpi-swa/smalltalkCI and from terminal execute something like:

./bin/smalltalkci -s Pharo64-12 /home/esteban/dev/repo/pharo-vcs/iceberg/.smalltalk.ston

Well, when I do that, I get an error because the CI image cannot find Sindarin ...

@adri09070
Copy link
Contributor Author

adri09070 commented Jan 23, 2024

Everything should be good once pharo-spec/Spec#1511 and pharo-project/pharo#16016 will be merged

@adri09070 adri09070 closed this Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need more work This needs to be improved before being considered
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debugger Tests are breaking the build
3 participants