fix(playwright): Add playwright workaround within Accordion's .expect_open() and .expect_panels()

[joesho112358]

The goal of this is to remove the early return from the accordion test so that more functionality is tested. Looks like an infinite loop may have been being introduced in the code that is commented. I am not sure what the intention of that block was, and can remove if not needed instead of commenting it.

The specific infinite loop in playwright:
https://github.com/microsoft/playwright-python/blob/main/playwright/_impl/_sync_base.py#L110

[schloerke]

The goal of expect_locator_values_in_list is to make sure that the attr values found (e.g. data-value) on matching elements are the only values found. Order matters.

Ideally, playwright would have an expect(loc).to_have_attribute(key, array_of_values) support, but they do not. 😞

I'd like to keep the expectation (below in the try/except) as a single expectation, rather than a loop of expectations (which would be MUCH easier to reason about). (If a failure occurs, we do loop over the different possibilities in an effort to debug the situation. Time does move forward while the expectation loop churns, allowing Shiny being able to update the accordion in between tests is not ideal.)

---

So unfortunately, I'd like to keep this locator creation code as it was. Thank you!

[joesho112358]

Hi, I am not sure I fully understand the reason to not loop through with the built in to_have_attribute(key, value). Could you elaborate on that?

[schloerke]

Time does move forward in the DOM when performing multiple expectations in a loop.

It is possible for Shiny to update the items in the list in question while in the middle of the for loop of expectations. So in the first have it is listA and in the back half it is listB. This makes debugging this rare situation really hard.

Also detecting items that are not in the list becomes difficult. I wouldn't know how to check for it as I wouldn't be able to guarantee that the K items found are the original set of K we're looking for.

---

To get around this difficulty, I use a loop to construct a very complicated locator. If this locator exists, then we can guarantee that the items in the list are the only items and their order is preserved.

Pseudo code for an expect(els_loc).to_have_attribute(attrs):

• for each el, make sure the corresponding attr matches.
• find all of the matching els; Verify the count is the expected count

If we can do this in a single expectation, then the expectation is playwrite-like in that it can be repeated until the expectation is satisfied.

If we can create another single expectation that achieves the same logic and doesn't block forever in the accordion test, I'm all for it!

[joesho112358]

I see, thanks for the explanation. Will think about it!

[schloerke]

@joesho112358 Any idea on how to stop the inf loop? (While keeping the locator creation as it is)

[joesho112358]

@schloerke Hi, I will look into it a bit more- I did narrow it down to this being the culprit:

            # Given the container, make sure it contains this locator
            loc_container = loc_container.locator(
                # Return self
                "xpath=.",
                has=has_locator,
            )

But why it fails specifically here is still unknown. I suspect it is due to the manner in which these elements are being added to the DOM.

[joesho112358]

@schloerke Hi, ready for another look. I had the thought that if we want everything all at once, another option is to construct 1 xpath that can handle it. Thoughts? Will still need to check some styling stuff though if you think this is an acceptable path.

[schloerke]

@joesho112358 Yes, happy to use xpath or locators.

However, the current solution does not assert the order is fixed and does not make sure the set is complete. Ex: DOM items A, A, D, C would pass when looking for A, B, C, D.

[joesho112358]

Closing this PR as I have not made progress and do not see a clear path forward

[joesho112358]

@schloerke hi again, this was bugging me a bit over the past few months. sorry if i repeat what was above here, but i have determined it is not the selector that is the issue. for some reason to_have_count goes into an infinite loop with this selector... so i worked around it because the count does not have that issue. it's a bit dirty, so if we need to close this again, then no worries!

[schloerke]

@joesho112358 We've given it our fairest shake to resolve the issues and we can not.

py-shiny does use multiple assertions in other pieces of code. While not ideal, it isn't too far of a stretch to perform many assertions here as well. I'd rather have ok coverage than missing coverage.

Thank you for resurrecting this!

[joesho112358]

@schloerke sorry the resurrection was so unrefined! I probably should have put it as a draft