-
Notifications
You must be signed in to change notification settings - Fork 25
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
Record XrSim tests for all the sample projects #170
Conversation
d75d8e5
to
c1c59b3
Compare
Question: Should we use Git LFS for the *.vrs files? They don't get that big, but if you make one that takes more than ~2 screenshots, they can get big-ish. The one for the passthrough sample on this PR is 57mb (I think I took 5 screenshots?). If we make a couple of revisions to that, it'll start to add up in the overall repo size when doing a full clone. The other *.vrs files are smaller: 5.9mb and 16mb (I only took 1 or 2 screenshots) I'm personally leaning towards using Git LFS, but since we aren't using it at all in this repo yet, I wanted to run it by other folks first! |
c3365e4
to
5524422
Compare
Seeing that any .vrs files we add potentially need to be updated if the related sample/feature is changed, and that "more than ~2 screenshots" feels like an extremely low bar to hit for having a large file, I'd agree that git LFS seems like the right choice. |
070263f
to
0a7a922
Compare
7246ccc
to
34841b3
Compare
8ed05cd
to
00cbc4a
Compare
Finally taking this one out of draft! I've got simple tests recorded for all the sample projects, except for the Scene API one: it works great when running it manually, but has some issue when running automatically - I suspect an XR Simulator bug, but I'm not really sure. In any case, it's something we can solve in a follow-up, rather than delaying on getting all the rest in. This PR also includes some good refactoring and improvements of the CI for running these tests. And I did end up moving the VRS files into Git LFS. |
00cbc4a
to
2eba8c9
Compare
2eba8c9
to
831119f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Now that we have tests via the Meta XR Simulator, this PR is adding tests for the two sample projects that are currently in the repo.
Some notes:
I had to downgrade to v64 of the simulator because passthrough wasn't workingUPDATE: These issues are fixed in v66, but it has it's own issues. I'm just going to use v65, even though passthrough isn't quite right - when there is a fully fixed version, we can re-record the VRS file.This is a draft since I don't think the CI is going to work on the first try because I reorganized some things. :-) Once it's working, I'll take it out of draft. (Also, I'm temporarily basing this on PR #149 to make iteration faster.)