Skip to content

Conversation

Devmoni
Copy link

@Devmoni Devmoni commented Mar 18, 2025

I’ve ported the activity to Python 3, please have a look!

I’ve been using flake8 to test and am also manually running it in Sugar to check its functionality.

Are there any additional tests or best practices you would recommend to ensure everything works correctly? Any guidance would be greatly appreciated for this and future ports.

@quozl

Thanks.

@quozl
Copy link
Contributor

quozl commented Mar 18, 2025

Your commit message does not follow our guidelines,

flake8 is not a test.

Above all, you should have a working Python 2 environment to test the current master branch. Use Sugar Live Build or OLPC OS 20.04 in a virtual machine. You must make sure the activity works before porting, and to establish a baseline of features and behaviours. Failing to do this wastes a heap of time.

@Devmoni
Copy link
Author

Devmoni commented Mar 19, 2025

  • Checked out my port branch.
  • Ran a new instance in Sugar.
  • Resolved all syntax errors, updated file paths, and fixed deprecated imports.
  • Tested the activity after porting to Python 3
  • Now it's working just like the master branch.

Currently, no TypeErrors are appearing in the console, and based on my testing, the activity behaves the same as before.

Here are the screenshots of the activity running in Sugar.

Screenshot from 2025-03-20 01-22-34

Screenshot from 2025-03-20 01-43-25

@quozl
Copy link
Contributor

quozl commented Mar 22, 2025

Thanks. Have used this opportunity to write sugarlabs/sugar#991 (comment) ... did you also observe these things before porting?

Tested c8afd0f;

  • many tracebacks in log, two different AttributeError,
  • the 12 and 16 block buttons don't respond.

@Devmoni
Copy link
Author

Devmoni commented Mar 23, 2025

Yes, I also observed these issues.

Regarding the tracebacks, I used them as a reference while porting on Ubuntu 20, but I will check again to confirm.

As for the 12 and 16 block buttons, they worked fine with the complete code during my testing.

Untitled.design.mp4

Thanks.

@quozl

@quozl
Copy link
Contributor

quozl commented Mar 28, 2025

Thanks. I'd like these fixed before merging this pull request.

@Devmoni
Copy link
Author

Devmoni commented Mar 28, 2025

Thanks. I'd like these fixed before merging this pull request.

Sorry, but I am not seeing any AttributeError in my testing. If you could share the specific error messages, it would be very helpful in understanding the issue better. Thanks!

@quozl
Copy link
Contributor

quozl commented Mar 28, 2025

Please describe exactly what your testing is, so we can see how it is different to mine.

I've checked out c8afd0f and tested again, like this;

  • click on uncoloured icon on the Sugar Home View,
  • exercise each app UI element,
  • click on stop,
  • click on coloured icon for the app on the Sugar Home View (i.e. resume from Journal),
  • exercise each app UI element,
  • click on stop,
  • review all logs, both shell.log and org.worldwideworkshop.olpc.SliderPuzzle*.log

The AttributeError is reported in the activity source code. It is something you've missed in your porting. If I copy and paste it here, we'll never find out why you didn't get it. 😁

Have you used any of the Python coverage testing?

- Replaced cStringIO with io.
- Used hashlib instead of md5, as md5 is no longer available as a standalone module.
- Fixed DBus import issues to align with Python 3 changes.
- Updated JSON handling by replacing json.write() with json.dumps() for proper serialization.
- Modified the exec command for correct execution in Python 3.
@Devmoni
Copy link
Author

Devmoni commented Mar 29, 2025

Thanks for the clarification! Initially, I was only checking the Sugar terminal logs, which is why I missed the issue. After following your testing method, I was able to catch the errors.

I also tested all the files separately by executing them, and so far, they seem to be working fine.
Additionally, I’ve updated mmm_modules for Python 3.

For coverage testing, I’ve now used it, but there are still some lines left untested.

Please test the latest commits.

Thanks.

@quozl
Copy link
Contributor

quozl commented Mar 30, 2025

You've committed a backup file.

@quozl
Copy link
Contributor

quozl commented Mar 30, 2025

Please also compare with sugarlabs/jigsaw-puzzle-branch ... perhaps your changes here will be useful there. The activities may be identical, I've not checked.

@Devmoni
Copy link
Author

Devmoni commented Apr 2, 2025

Please also compare with sugarlabs/jigsaw-puzzle-branch ... perhaps your changes here will be useful there. The activities may be identical, I've not checked.

Thanks!
Please review this PR from your end as well.
And I’m currently working on the sugarlabs/jigsaw-puzzle-branch, porting the activity from Gtk2 to Gtk3 and Python 3. I’ll submit the PR as soon as possible.

Let me know if there’s anything else that needs attention.

@quozl
Copy link
Contributor

quozl commented Apr 2, 2025

Reviewed. Looks fine. Please do make a corresponding change to jigsaw-puzzle-branch, so as to avoid having someone change that independently of this. Where code is copied into other activities, as had happened here, we waste developer resources if we don't change copies at the same time. Thanks.

@quozl quozl self-requested a review April 2, 2025 21:23
@Devmoni
Copy link
Author

Devmoni commented Apr 3, 2025

I’ll update jigsaw-puzzle-branch to keep things consistent.
Thanks for the guidance!

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.

2 participants