8000 Add a selector to the HTML tag + export config by WebReflection · Pull Request #1773 · pyscript/pyscript · GitHub
[go: up one dir, main page]
Skip to content

Add a selector to the HTML tag + export config #1773

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 29, 2023
Merged

Add a selector to the HTML tag + export config #1773

merged 1 commit into from
Sep 29, 2023

Conversation

WebReflection
Copy link
Contributor
@WebReflection WebReflection commented Sep 29, 2023 •
edited
Loading

Description

This MR adds an easy to wait for HTML class selector on py:all-done event + it exports pyConfig and mpyConfig globally to let us easily test anything we'd like to test.

Changes

  • leak globally config.py as pyConfig and config.mpy as mpyConfig
  • use the HTML selector to be sure DOM changes are applied

Checklist

  • All tests pass locally
  • I have updated docs/changelog.md
  • I have created documentation for this(if applicable)

@antocuni
Copy link
Contributor

Something is very strange.
The current code already waits until py:all-ready is triggered. These are the relevant lines:

  1. every call to pyscript_run calls wait_for_pyscript()
    https://github.com/pyscript/pyscript/blob/fb2532cd13dbafa066146ac499317e6fca8c98a9/pyscript.core/tests/integration/support.py#L543-L544

  2. wait_for_pyscript waits for the ---py:all-ready-- message on the console https://github.com/pyscript/pyscript/blob/fb2532cd13dbafa066146ac499317e6fca8c98a9/pyscript.core/tests/integration/support.py#L465-L473

So, I'm not really sure how this PR can turn flaky tests into non-flaky tests. Maybe they are still flaky but they haven't failed yet? 🤔

I am not opposed to use a CSS selector instead of a console message, but this should be done automatically by run_pyscript(), not required to do manually in every tests.

@WebReflection
Copy link
Contributor Author

My gut feeling is that console resolves before changes happen or are applied to the DOM ... and as we instead check the DOM, being sure that the DOM has updated, not the console stream just logged, feel the right thing to do.

I will change this MR to reflect this idea and we can then see if we like it or not, thanks for the pointers!

antocuni
antocuni reviewed Sep 29, 2023
View reviewed changes
antocuni
antocuni approved these changes Sep 29, 2023
View reviewed changes
Copy link
Contributor
@antocuni antocuni left a comment

Choose a reason for hiding this comment

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

LGTM

@WebReflection
Copy link
Contributor Author

To whom it might concern: this MR was supposed to fix some flaky test result (most notably on the display multiple test case) but CI backfired and needed another run ... we're investigating why that's the case but at least we have easier way to parse config and we removed an ugly comment and alien code we couldn't understand so ... half win, which is still more a win than a fail 😅

I am going to merge this but further investigation on the root cause is ongoing.

@WebReflection WebReflection merged commit fdc35ce into pyscript:main Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antocuni antocuni antocuni approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WebReflection @antocuni
0