fix: collect all zone validation errors instead of failing on first (#1095)

[szichedelic]

Description

This PR fixes #1095 by modifying the zone parser to collect and report all validation errors at once, rather than stopping at the first error.

Changes

• Modified parse_zones() to collect errors from all lines
• Modified Scene::parse_from_zone() to accumulate all validation errors
• Added comprehensive unit tests for the new behavior

Example

Previously, users would see only the first error:
Error: Start frame must be earlier than the end frame

Now they see all errors with context:
Zone file validation failed with 3 error(s):

Line 2 "20 15 aom --cq-level=25":
Start frame must be earlier than the end frame

Line 3 "30 150 aom --cq-level=35":
Start and end frames must not be past the end of the video

Line 4 "40 50 x264 reset":
Zone specifies using x264, but this cannot be used in the same file as aom

This allows users to fix multiple issues in their zone files in a single pass.

Fixes #1095

[codecov]

Codecov Report

❌ Patch coverage is 81.87919% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.37%. Comparing base (0fcc2a8) to head (1572604).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
av1an-core/src/scenes/mod.rs	38.63%	27 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1098      +/-   ##
==========================================
+ Coverage   60.79%   61.37%   +0.57%     
==========================================
  Files          23       23              
  Lines        6316     6447     +131     
==========================================
+ Hits         3840     3957     +117     
- Misses       2476     2490      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull Request Overview

This PR modifies the zone file parser to collect and report all validation errors at once instead of failing on the first error, improving the user experience by allowing them to fix multiple issues in a single pass.

Key changes:

• Enhanced error collection to accumulate validation errors from all lines before reporting
• Modified both parse_zones() and Scene::parse_from_zone() to collect multiple errors
• Added comprehensive unit tests to verify the new error collection behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
av1an-core/src/zones.rs	Modified parse_zones() to collect errors from all zone lines and added unit tests for multi-error collection
av1an-core/src/scenes/mod.rs	Enhanced Scene::parse_from_zone() to accumulate all validation errors instead of early returns
av1an-core/src/scenes/tests.rs	Added test case to verify multiple error collection in zone parsing

[shssoichiro]

Looks good, just needs one formatting cleanup based on the Copilot review, as well as a merge conflict resolution. Thanks!

[BoatsMcGee]

Thanks for the quick turnaround on this one. I'm looking into resolving the conflicts myself since it's related to my recent PR that also modifies the same parser function. I should have mentioned in the first place that my Issue was related to my PR so that's my mistake. I also want to handle warnings for both success and error cases so they don't just print as they're encountered but are also collected and returned. Lastly, the tests added to the zone.rs should be refactored into /zones/tests.rs. I'll post a diff/patch when I'm done with all that.

Thanks,
- Boats M.

[BoatsMcGee]

Here's a patch that adds warnings and refactors scenes.rs into /scenes/mod.rs and /scenes/tests.rs. I also fixed a Clippy issue with the test. I did not address Copilot's issue though - my only suggestion is that we should move away from writing to STDOUT and support feedback such that a library calling this function could handle it.

merge.patch (this patch might have included the whole PR as well. If it fails, try this next one: merge.patch)

[szichedelic]

@BoatsMcGee Thanks for the patch. I will get this cleaned up shortly here.

[BoatsMcGee]

Are the patches applying? If neither patch is working or it just makes it any easier for you, I could open a PR on your fork with the changes from my branch. You should have rights to modify my branch for whatever you need or you could just pull the changes, make your modfications, then merge and update the branch for this PR.

[szichedelic]

Are the patches applying? If neither patch is working or it just makes it any easier for you, I could open a PR on your fork with the changes from my branch. You should have rights to modify my branch for whatever you need or you could just pull the changes, make your modfications, then merge and update the branch for this PR.

No the patches aren't applying actually. I was going to just manually bring the changes in but have been bogged down with some other things today. I think I will just pull your changes in, if that's okay, that would be a lot easier.

[BoatsMcGee]

No problem, made the PR. Take all the time you need.

I think the first patch was faulty because it included changes from merging master in. The 2nd patch did work but I had to merge master in and commit before applying the patch. Sorry about that, I should have double-checked to make sure the patches applied to your branch rather than just my master-merged local branch.

[shssoichiro]

Looks like there are some failing unit tests, as well as some merge conflicts that will need resolved.