tests: update a11y artifacts #9010
Conversation
proto/sample_v2_round_trip.json
Outdated
| @@ -1115,7 +1115,7 @@ | |||
| "name": "WordPress" | |||
| } | |||
| ], | |||
| "summary": {}, | |||
There was a problem hiding this comment.
@exterkamp this is expected now, right? I've lost track of all the summary fixes and what's real and what's just tests 😆
There was a problem hiding this comment.
No. @brendankenny can get his proto to output {} instead of null, but I can't get mine to, and I guess @mattzeunert can't either. {} should be considered canonical imo, and we need to source out this difference.
There was a problem hiding this comment.
make sure you're using the latest protobuf compiler.
There was a problem hiding this comment.
I'm on protoc v=3.7.1
I just recompiled from source. I'm going to try to uninstall and reinstall.
There was a problem hiding this comment.
Ran brew upgrade protobuf to upgrade to 3.7.1 and it works fine for me now.
There was a problem hiding this comment.
No. @brendankenny can get his proto to output {} instead of null, but I can't get mine to, and I guess @mattzeunert can't either
I guess @exterkamp now stands alone with his buggy protoc :)
There was a problem hiding this comment.
{} is canonical for empty objects inside a struct like Audit.details and we think upgrading to protoc > 3.7.1 should use {}, but mine doesn't work. So I don't think the 'proto test' should have this conversion code. We need to sort out why ours aren't updating 🤕
proto/sample_v2_round_trip.json
Outdated
| @@ -1115,7 +1115,7 @@ | |||
| "name": "WordPress" | |||
| } | |||
| ], | |||
| "summary": {}, | |||
There was a problem hiding this comment.
No. @brendankenny can get his proto to output {} instead of null, but I can't get mine to, and I guess @mattzeunert can't either. {} should be considered canonical imo, and we need to source out this difference.
This reverts commit 985c2c9.
I think it was mostly from #8373 (with another one from rob that added |
brendankenny
dismissed
exterkamp’s stale review
requested proto change fixed, now on vacation :)
Summary
Update out of date a11y data. Thought it might be related to #8823, but that PR did update the the artifacts – not sure what changes these are.
Also, when running
npm run update:sample-jsonon master you get thesummary: nullsample change, which broke the proto tests. Some of the tests already had abeforeAllthat fixed that, so moved that beforeAll to be top-level.