Fix #617

@antocuni @marimeireles I think we are overlapping efforts here... I've started a PR to add JS unit tests and, also another one to add Python tests.

Given the skeleton of this PR I'm not sure I understand the goal (what it's testing) and if we are duplicating efforts, but just wanted to give you a heads up.

@antocuni @marimeireles I think we are overlapping efforts here... I've started a PR to add JS unit tests and, also another one to add Python tests.

I think they are not-overlapping but complementary. If I understand correctly:

• Add TS tests tooling #661 adds JS unit tests
• [WIP] bootstrap python tests #665 adds Python unit tests which are run outside the browser
• this PR wants to add integration tests to test what happens when you load a pyscript page inside the browser, but much more granular than e.g. examples.

Given the skeleton of this PR I'm not sure I understand the goal (what it's testing) and if we are duplicating efforts, but just wanted to give you a heads up.

yes, the PR is still very early so I agree it's hard to see where it's headed to without reading into our minds :). The idea is to have enough scaffolding and machinery to make writing small tests very easy and natural. For example, eventually I would like to be able to write something along these lines:

def test_py_button(self):
    src = """
    <py-button id="mybutton">
        def onClick():
            print('button clicked')
    </py-button>
    """
    self.write_page("mytest.html", src) # the rest of the HTML is automatically added around src
    page = self.open("mytest.html")
    page.locator('id=mybutton').click()
    assert self.console[-1] = "button clicked" # or something like that

Thanks for all the work Antonio! Is really great ✨ :)

@antocuni, I saw you changed from the two logs, one for "pyscript loading" and one for "pyodide loading" to only one "pyscript fully initialized".
The reason why I had these two logs is to make it easier to the debug. That problem I described to you and that we arrived to the conclusion that was happening because of the delay on the tests on checking the HTML, it'd have been a bit easier for me to debug if I knew exactly which part of pyscript wasn't loaded.
Maybe we can change the '===PYSCRIPT LOADED=== to '===PYSCRIPT FULLY LOADED===, if you think it's better.
What do you think about it? Do you have a different opinion on the number of logs here?

@antocuni, I saw you changed from the two logs, one for "pyscript loading" and one for "pyodide loading" to only one "pyscript fully initialized". The reason why I had these two logs is to make it easier to the debug. That problem I described to you and that we arrived to the conclusion that was happening because of the delay on the tests on checking the HTML, it'd have been a bit easier for me to debug if I knew exactly which part of pyscript wasn't loaded. Maybe we can change the '===PYSCRIPT LOADED=== to '===PYSCRIPT FULLY LOADED===, if you think it's better. What do you think about it? Do you have a different opinion on the number of logs here?

Right sorry, I should have talked to you before doing that.
My reasoning was the following:

1. in general, we should at some point rationalize our logging, because currently it's a mess; but that's not the job of this PR

2. ===PYSCRIPT LOADED=== was here:
   

   pyscript/pyscriptjs/src/interpreter.ts

   Lines 25 to 33 in 9957af0

   	console.log('loading pyscript...');
   	const output = await pyodide.runPythonAsync(pyscript);
   	if (output !== undefined) {
   	console.log(output);
   	}
   	console.log('done setting up environment');
   	console.log('===PYSCRIPT LOADED===');

   
   The new log doesn't add any new information, because it comes directly after done setting up environment. But more importantly, it's a lie :), because at that point in time pyscript has not finished loading yet, at least not in the expected sense IMHO. If you look further in the logs you see that there are still a lot of things happening in the page. I guess the confusion arises from loading pyscript... at line 25, but this should really be importing the pyscript module..., which is just a small fraction of the whole "loading pyscript".

3. Similarly, ===PYODIDE LOADED=== was here:

   pyscript/pyscriptjs/src/components/pyconfig.ts

   Lines 111 to 121 in 9957af0

   	// now we call all post initializers AFTER we actually executed all page scripts
   	loader?.log('Running post initializers...');
   	if (appConfig_ && appConfig_.autoclose_loader) {
   	loader?.close();
   	}
   	for (const initializer of postInitializers_) {
   	await initializer();
   	}
   	console.log('===PYODIDE LOADED===');

   
   But again, it's poorly worded: pyodide is fully loaded much earlier, at line 81. That's why I reworded to something more precise, and also more informative to the user which looks at the console.

A note for those who are following the conversation in this PR: I modified the first message, which now contains a full description of this PR: #663 (comment)

I reviewed "very fast" but overall it's a GREAT PR. Ton of value added and well documented. A couple of things that I think should be added/addressed later:

• would be good to add some timing to also have timestamps of how long things are taking (maybe it's not part of these tests but I wanted to use this say it out loud :) )

this is actually a very good idea, and was simple enough that I did in commit 18c8ec9.
Screenshot of running test_pyscript_hello:

• we'll probably need to restructure the pyscriptjs/tests folder a little and maybe put this in pyscriptjs/integration or something like this

As you might guess, I have opinions on that for fortunately not as strong as for other topics :).
But yes, I agree we can discuss it separately.

Since it seems there are no big objects, I'm going to merge it.