Skip to content

Conversation

omehes
Copy link
Contributor

@omehes omehes commented Sep 15, 2025

@omehes omehes requested a review from a team as a code owner September 15, 2025 21:05
@omehes omehes requested a review from Dantemss September 15, 2025 21:05
@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 15, 2025 21:06 Inactive
Copy link
Member

@Dantemss Dantemss left a comment

Choose a reason for hiding this comment

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

2 comments, the rest looks fine


assert home.footer_section_bottom_is_visible

assert "Rice University" and "license" in await home.footer_section_bottom_is_visible.inner_text()
Copy link
Member

Choose a reason for hiding this comment

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

The in operator has higher precedence than and so this is equivalent to assert "Rice University" and ("license" in await home.footer_section_bottom_is_visible.inner_text()) so the "Rice University" part is not testing anything. So you either need 2 different asserts or use in twice, like: "Rice University" in ... and "license" in ...

await home.click_learn_about_openstax_link()

assert f"{base_url}/about" == chrome_page.url
assert "Who we are" and "What we do" and "Where we're going" in await home.about_page.inner_text()
Copy link
Member

Choose a reason for hiding this comment

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

Same deal here

@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 16, 2025 16:16 Inactive
@omehes
Copy link
Contributor Author

omehes commented Sep 16, 2025

@Dantemss The fails are due to a "Make a gift" overlay appearing on staging.openstax... When rerunning them and getting rid of the overlay, they pass.

@omehes omehes requested a review from Dantemss September 17, 2025 13:24
@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 17, 2025 15:00 Inactive
@omehes omehes dismissed Dantemss’s stale review September 17, 2025 17:11

Fixed as per review

@omehes
Copy link
Contributor Author

omehes commented Sep 18, 2025

@Dantemss I think this still waits for your re-approval...

@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 19, 2025 17:17 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 19, 2025 19:20 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 22, 2025 15:58 Inactive
@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 22, 2025 19:39 Inactive
@omehes
Copy link
Contributor Author

omehes commented Sep 23, 2025

@Dantemss fixed the issues. Thanks for pointing them out

@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 23, 2025 20:44 Inactive
Copy link
Member

@Dantemss Dantemss left a comment

Choose a reason for hiding this comment

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

Is there a reason some of them use async/await and others don't? I didn't see anything that seemed to imply the async/await was needed

def philanthropic_support_section(self):
return self.page.locator("section.philanthropic-support")
@pytest.mark.asyncio
async def philanthropic_support_section(self):
Copy link
Member

Choose a reason for hiding this comment

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

This one maybe should end in _is_visible

def footer_section(self):
return self.page.locator("id=footer")
@pytest.mark.asyncio
async def footer_section(self):
Copy link
Member

Choose a reason for hiding this comment

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

same here

@TomWoodward TomWoodward temporarily deployed to rex-web-osweb-dqqhxelns64aaz2i September 24, 2025 17:38 Inactive
@omehes omehes merged commit faa62dd into main Sep 24, 2025
7 of 8 checks passed
@omehes omehes deleted the osweb branch September 24, 2025 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants