Skip to content
This repository has been archived by the owner on Feb 8, 2021. It is now read-only.

Add test to dynamically create XRPackages by adding/removing files & Improve test suite assets #105

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

shu8
Copy link
Contributor

@shu8 shu8 commented Jul 29, 2020

This PR adds a test to make sure the addFile and removeFile methods of XRPackage work, by trying to dynamically package multiple files starting from an empty XRPackage instance.

Note: I've had to tweak addFile for this test to work, so we now support adding files to an an empty XRPackage. This seems like it should be something we support, because we already allow instantiating an XRPackage with empty data, and the main thing we'd want to do with an empty XRPackage is add files to it.

Note: I've also fixed the webxr template asset which had broken imports (closes #119). It's now using the simple example scene from https://threejs.org/docs/index.html#manual/en/introduction/Creating-a-scene, with a few modifications. I've changed the baked-xrpk and unbaked-xrpk tests to use this wbn as well, which gives us a smaller file size to test with.

Part of #64.

@shu8 shu8 force-pushed the shu8/dynamically-creating-xrpackages-tests branch from 66c1961 to 1cde8ef Compare July 29, 2020 15:34
xrpackage.js Show resolved Hide resolved
shu8 added 5 commits July 30, 2020 11:40
Previously, wbn.Bundle(this.data) threw an exception if this.data was
empty.

As we already support instantiating an XRPackage with no data, we should
support addFile() with no data too.
- Use minified bundled version of very simple THREE scene (save bytes)
- Also adds source JS for scene in case it needs modifying

Minifed bundled version built with Parcel.
@shu8 shu8 force-pushed the shu8/dynamically-creating-xrpackages-tests branch from 8c55691 to 4a044b7 Compare July 30, 2020 11:55
@shu8 shu8 changed the title Add test to dynamically create XRPackages by adding/removing files Add test to dynamically create XRPackages by adding/removing files & Improve test suite assets Jul 30, 2020
shu8 added 2 commits July 31, 2020 12:22
This ensures that if the user isn't adding a manifest, there is a
manifest.json in the wbn, so calls to the `wbn` methods don't throw an
error.
@shu8 shu8 requested a review from avaer July 31, 2020 11:25
@@ -0,0 +1,6 @@
<!DOCTYPE html><html lang="en"><head><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=edge"><meta name="viewport" content="width=device-width, initial-scale=1"><title>WebXR Cube</title></head><body> <script type=module>var parcelRequire=function(e,r,t,n){var i,o="function"==typeof parcelRequire&&parcelRequire,u="function"==typeof require&&require;function f(t,n){if(!r[t]){if(!e[t]){var i="function"==typeof parcelRequire&&parcelRequire;if(!n&&i)return i(t,!0);if(o)return o(t,!0);if(u&&"string"==typeof t)return u(t);var c=new Error("Cannot find module '"+t+"'");throw c.code="MODULE_NOT_FOUND",c}p.resolve=function(r){return e[t][1][r]||r},p.cache={};var l=r[t]=new f.Module(t);e[t][0].call(l.exports,p,l,l.exports,this)}return r[t].exports;function p(e){return f(p.resolve(e))}}f.isParcelRequire=!0,f.Module=function(e){this.id=e,this.bundle=f,this.exports={}},f.modules=e,f.cache=r,f.parent=o,f.register=function(r,t){e[r]=[function(e,r){r.exports=t},{}]};for(var c=0;c<t.length;c++)try{f(t[c])}catch(e){i||(i=e)}if(t.length){var l=f(t[t.length-1]);"object"==typeof exports&&"undefined"!=typeof module?module.exports=l:"function"==typeof define&&define.amd?define(function(){return l}):n&&(this[n]=l)}if(parcelRequire=f,i)throw i;return f}({"noWu":[function(require,module,exports) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this file parcelified? Is it being uploaded somewhere?

I could understand if this was being uploaded, in which case file size matters so we don't waste bandwidth. But otherwise I think we should stick to unminified test code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not being uploaded in this test case, but my thinking was that we could use this single .wbn asset for other tests that might be uploading stuff rather than creating another another one that's minified.

I also parcelified it because it's easier to write tests that package WebXR apps with a single file like compileFromFile when packaging a single file (

test('compile xrpk from html', withPageAndStaticServer, async (t, page) => {
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, in that case could we parcel it without this minification? Basically it's impossible to edit this HTML in this form.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just pushed a change to unminify the JS and HTML!

Copy link
Contributor

@avaer avaer left a comment

Choose a reason for hiding this comment

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

Just the one question about unminified HTML.

This makes it easier to modify and read, whilst keeping the parcelled
THREE code in the same file so we can still use a single file in the
tests.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Test] webxr-template.html imports unknown files
2 participants