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

update: make tests pass again; fixes for lingering small items #607

Merged
merged 17 commits into from
May 2, 2024
Merged

update: make tests pass again; fixes for lingering small items #607

merged 17 commits into from
May 2, 2024

Conversation

hanbollar
Copy link
Contributor

@hanbollar hanbollar commented May 2, 2024 •
edited
Loading

Linking

related to #606
Fixes #602 and other things exposed by 606

Required to Merge

  • PASS - all necessary actions must pass (excluding the auto-skipped ones)
    - [ ] TEST IN HEADSET - main dev-testing-example and any of the other examples still work as expected
    - [ ] VIDEO - if this pr changes something visually - post a video here of it in headset-MR and/or on desktop (depending on what it affects) for the reviewer to reference.
  • TITLE - make sure the pr's title is updated appropriately as it will be used to name the commit on merge
    - [ ] BREAKING CHANGE
    • DOCUMENTATION: This includes any changes to html tags and their components
      • make a pr in the documentation repo that updates the manual docs to match the breaking change
      • link the pr of the documentation repo here: #pr
      • that pr must be approved by @lobau
    • SAMPLES/INDEX.HTML: This includes any changes (html tags or otherwise) that must be done to our landing page submodule as an effect of this pr's updates
      • make a pr in the mrjs landing page repo that updates the landing page to match the breaking change
      • link the pr of the landing page repo here: #pr
      • that pr must be approved by @hanbollar

@hanbollar
start
70d742a 
Signed-off-by: hanbollar <[email protected]>
@render Render
Copy link

render bot commented May 2, 2024

Your Render PR Server URL is https://examples-mrjs-pr-607.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-copf9nuv3ddc73fob3v0.

@hanbollar
some fixes
e4054a3 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
fix video/image
4860a2d 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
temp save
699623a 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
Merge branch 'main' into hb-fix
976f125
@hanbollar
temp save
b3331ed 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
quick
98c0c1b 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
update
afe6abe 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
temp save
55903a3 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
Copy link
Contributor Author

hanbollar commented May 2, 2024 •
edited
Loading

also found:

weird thing in the images.html example - i think it's a lingering background from something // maybe from the different scaling options where one of the 'quick fix' intermediary backrounds isnt getting deleted --> not sure where that's coming from atm, but not a super big blocker

Screenshot 2024-05-02 at 3 27 57 PM

#609

@hanbollar
update testing
bf52ee2 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
some notes
4c7d9ff 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
add way to clear jest cache
b07007d 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
comment out
f55c9f1 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
cleanup
28b452d 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
cleanup
ea172a1 
Signed-off-by: hanbollar <[email protected]>
@hanbollar hanbollar changed the title WIP - update: make tests pass again; fixes for lingering small items update: make tests pass again; fixes for lingering small items May 2, 2024
@hanbollar
Copy link
Contributor Author

hanbollar commented May 2, 2024 •
edited
Loading

lingering notes:

^ this is causing extra errors to occur, for ex: MRSystem not existing, even though it does, just not in the puppeteer setup at that time -- these are only erroring due to an improper testing setup, so commenting it out - to noodle on a little still

  • scripts/check-and-update-submodule.sh only runs nicely on main, not a branch

going to open an issue based off this to resolve, but not a high priority

@hanbollar hanbollar mentioned this pull request May 2, 2024
Open
@hanbollar
update
c9aa061 
Signed-off-by: hanbollar <[email protected]>
@hanbollar
update
ca96ac8 
Signed-off-by: hanbollar <[email protected]>
@hanbollar hanbollar mentioned this pull request May 2, 2024
Open
hanbollar
hanbollar commented May 2, 2024
View reviewed changes
__tests__/checkSubmoduleTesting.test.js
Comment on lines +23 to +25
// Need to comment out this testing file as it causes an improper merge/stash/branch setup when run with
// the rest of the tests automatically. Will bring it back as part of #608 being resolved
console.log('SKIPPING THIS FOR NOW - see pending issue: https://github.com/Volumetrics-io/mrjs/issues/608');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

calling out this comment for why this file was changed in this pr

hanbollar
hanbollar commented May 2, 2024
View reviewed changes
__tests__/examplesTesting.test.js
Comment on lines +26 to +29
// page.on('pageerror', error => {
// errors.push(error.toString());
// console.error(`Unhandled error: ${error}`);
// });
Copy link
Contributor Author

@hanbollar hanbollar May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this is the part that is erroring on items that only error in puppeeter, to be resolved in #608 , not priority here

hanbollar
hanbollar commented May 2, 2024
View reviewed changes
scripts/check-and-update-submodule.sh
@@ -46,26 +46,36 @@ if [ "$#" -ne 1 ]; then
exit 1
fi

echo "HI4"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since this file isnt being used by any runner (since the test is commented out, leaving this in as i debug this over time, no harm no foul

hanbollar
hanbollar commented May 2, 2024
View reviewed changes
scripts/update-all-submodules.sh
Comment on lines +77 to +81
echo "HI"
SUBMODULE_DIR="samples/mrjsio"
echo "HI2"
./scripts/check-and-update-submodule.sh "$SUBMODULE_DIR"
echo "HI3"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ditto to https://github.com/Volumetrics-io/mrjs/pull/607/files#r1588510738

@hanbollar
Copy link
Contributor Author

merging in since this was mostly a testing process fix and like 1 or 2 one-liners

@hanbollar hanbollar merged commit 9465bcf into main May 2, 2024
6 checks passed
@hanbollar hanbollar deleted the hb-fix branch May 2, 2024 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

video/media and image example have a bugs
1 participant
@hanbollar