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.

Sign up for GitHub

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

Jump to bottom

Fix seek_stress test papercut #880

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Fix seek_stress test papercut #880

merged 2 commits into from
Mar 21, 2024

Conversation

thedataking
Copy link
Collaborator

@thedataking thedataking commented Mar 21, 2024 •
edited by kkysen
Loading

folkertdev
folkertdev reviewed Mar 21, 2024
View reviewed changes
Copy link
Collaborator

@folkertdev folkertdev left a comment

Choose a reason for hiding this comment

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

nice! I verified that this works locally

with that, to close the issue, I think the readme should point to the test.sh script instead of a manual meson command. It's shorter, and should always mirror CI (e.g. if more test suites are added)

just this could work

Then you can run the tests with:

.github/workflows/test.sh -r target/release/dav1d

@thedataking
Copy link
Collaborator Author

I think the readme should point to the test.sh script instead of [...]

Sure. I took a pass over the README.md, let me know what you think.

randomPoison
randomPoison approved these changes Mar 21, 2024
View reviewed changes
Copy link
Collaborator

@randomPoison randomPoison left a comment

Choose a reason for hiding this comment

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

Ah thanks for fixing this, I'd been commenting out the seek_stress line in order to test locally.

kkysen
kkysen approved these changes Mar 21, 2024
View reviewed changes
Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

This is helpful, thanks! I had been thinking we should say to use test.sh in the README.md, too.

@thedataking thedataking merged commit 400cb1d into main Mar 21, 2024
36 checks passed
@thedataking thedataking deleted the perl/issue-873 branch March 21, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@randomPoison randomPoison randomPoison approved these changes

@folkertdev folkertdev folkertdev approved these changes

@kkysen kkysen kkysen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tests fail to link
4 participants
@thedataking @randomPoison @folkertdev @kkysen