Skip to content
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.

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

Migrate typography-cypress test to playwright #8594

Merged
merged 6 commits into from
May 3, 2024

Conversation

raethlein
Copy link
Collaborator

@raethlein raethlein commented May 2, 2024

Describe your changes

This PR migrates the existing cypress test "typography" to playwright. The test code and cases are merged into the st_markdown test as that test file bundles the different markdown scenarios already.
In this context, the existing st_markdown test is cleaned up a little bit to not double-test headers etc.

The migration happens in preparation of the #8587 PR. In that PR a couple of screenshots need to be updated but I don't want to touch the legacy cypress test artifacts.

GitHub Issue Link (if applicable)

Testing Plan

  • ✅ E2E Tests

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

Comment on lines -39 to -41
st.markdown("# Some header 1")
st.markdown("## Some header 2")
st.markdown("### Some header 3")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed these because in the bottom of this file we have a couple of different header tests now, so these are not needed anymore

@@ -55,40 +51,10 @@
":green-background[LaTeX math within green background: $ax^2 + bx + c = 0$]"
)

with st.container():
st.markdown("# some really long header " + " ".join(["lol"] * 10))
Copy link
Collaborator Author

@raethlein raethlein May 2, 2024

Choose a reason for hiding this comment

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

moved this test down to the other header tests

Comment on lines +100 to +179
def draw_header_test(join_output):
strings = [
"# Header header1",
"## Header header2",
"### Header header3",
"#### Header header4",
"##### Header header5",
"###### Header header6",
"Quisque vel blandit mi. Fusce dignissim leo purus, in imperdiet lectus suscipit nec.",
]

if join_output:
st.markdown("\n\n".join(strings))
else:
for string in strings:
st.markdown(string)


draw_header_test(True)

with st.sidebar:
st.text_input("This is a label", key="1")
draw_header_test(True)

"---"

with st.container():
st.text("Headers in single st.markdown command")
draw_header_test(True)

"---"

with st.container():
st.text("Headers in multiple st.markdown command")
draw_header_test(False)

"---"

with st.container():
st.text("Headers in columns")

a, b = st.columns(2)

with a:
draw_header_test(True)

with b:
draw_header_test(False)

"---"

with st.container():
st.text("Headers in columns with other elements above")

a, b = st.columns(2)

with a:
st.text("This is some text")
draw_header_test(True)

with b:
st.text("This is some text")
with st.container():
draw_header_test(False)

"---"

with st.container():
st.text("Headers in column beside widget")

a, b = st.columns(2)

with a:
st.write("# Header header")
st.write("## Header header")

with b:
st.text_input("This is a label", key="2")

"---"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the existing typography test code that is migrated in this PR from Cypress

@raethlein raethlein changed the title Tests/migrate typography to playwright Migrate typography-cypress test to playwright May 2, 2024
@raethlein raethlein marked this pull request as ready for review May 2, 2024 13:55
@raethlein raethlein force-pushed the tests/migrate-typography-to-playwright branch 2 times, most recently from 277bcbe to 836f29a Compare May 2, 2024 14:40

# take the 2nd match because the first would be the most outer block
return (
app.get_by_test_id("stVerticalBlock")
Copy link
Collaborator

Choose a reason for hiding this comment

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

clever :) But would be even simpler if we could add a class to a container via key. Hopefully, we can add that soon.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes!

Copy link
Collaborator

@LukasMasuch LukasMasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍 But I think it would be good to remove some of the headers from the large markdown snapshot

@raethlein raethlein force-pushed the tests/migrate-typography-to-playwright branch from a97b197 to 6444500 Compare May 3, 2024 08:31
@raethlein raethlein merged commit d93a282 into develop May 3, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants