Question: since acceptDataUseDownloadDialog is an async func, I believe this func need to be async like const handleDialog = asyc (d) => await this.acceptDataUseDownloadDialog(treePage, d);?

You need the async keyword if you want to use await in your function, that's it. In this case, there is no need to use the await keyword, therefore we don't need it to be an async function. What you've written is functionally equivalent to what I wrote, but with extra steps.

It will be difficult to explain this in a comment - lets stay on and talk through it after eng sync, if you want.

suggestion: treePage.once is a good alternative to treePage.on and treePage.off.

Good find, thanks - I hadn't seen once. That said, on/off is slightly more hermetic. There is no guarantee that your handler actually gets called within the scope of this method, e.g. if the alert stopped showing up in certain cases, or there was a product change that removed it. If that were to occur, with once you would be leaking a dialog listener into unrelated sections of the test.

once/off may be an improvement here. I'm also thinking the off should be further down.. I'm slightly concerned now about a race condition between the dialog showing up, and the off call.

Another alternative would be once + assert that your listener was invoked. That could be the best option if what we're trying to do here is actually test the modal behavior.

👍🏻 I like once + assert too.

I switched to this, and it exposed the fact that indeed we're not always getting the modal on upload/download. I suspect this is a race condition between clicking upload, and when the Jupyter extension (which adds the modal) is installed. I added another unfortunate sleep here to cover this situation.

Open to other suggestions. I also filed https://precisionmedicineinitiative.atlassian.net/browse/RW-8144 which will give generic support for page.waitForNetworkIdle(), which is something I wanted to try here.

I ran this test locally several times. 1 time it had failed. Unfortunately, after e.click((), it needs a short sleep for improved event synchronization.

Thanks for testing this out locally. Do you still have the test logs from when you saw this error?

Also, are you sure it was this line and not L143? I see a pretty clear potential race condition there.

Not anymore. It was overwritten after I made few local changes. I thought it could happen again if I rerun again. But failure only happened once.

yes, it was at line 143: after click Download button. my comment was at the wrong line.

suggestion: Use newPage.once.