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

OPIK-992 automation fixes #1274

Merged
merged 12 commits into from
Feb 14, 2025
Merged

Conversation

AndreiCautisanu
Copy link
Contributor

@AndreiCautisanu AndreiCautisanu commented Feb 13, 2025

Details

Various automation fixes for small changes to UI in prompt/dataset item tables, also more fixes for ensuring that tests run properly on cloud environments as well.

Pagination buttons issue turned out to be a problem with the newest version of Playwright, where some newly added checks for is_editable() now get called for is_enabled() too, rendering is_enabled() unusable on buttons. Reverted the playwright version and forcing it for now, will raise an issue on the Playwright repo.

@AndreiCautisanu AndreiCautisanu requested review from a team as code owners February 13, 2025 11:48
@AndreiCautisanu AndreiCautisanu requested a review from a team as a code owner February 14, 2025 08:24
andrescrz
andrescrz previously approved these changes Feb 14, 2025
Copy link
Collaborator

@andrescrz andrescrz left a comment

Choose a reason for hiding this comment

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

Just a few minor comments. But this is good to go.

Comment on lines +334 to +335
except Exception as _:
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

You probably want to log at info level, so we can better debug it if something is unexpected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these throwing an exception would not necessarily indicate something unexpected happening, just that it does not need to clean up the created database because it was deleted as part of the test. that said, i will be adding logging fixtures in my next ticket and documenting this too, so staying with this as is for now and adding loggers in the next PR.

@AndreiCautisanu AndreiCautisanu merged commit a5801dd into main Feb 14, 2025
2 checks passed
@AndreiCautisanu AndreiCautisanu deleted the andrei/OPIK-992/automation-fixes branch February 14, 2025 10:27
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.

2 participants