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.

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

Support a default HTML wrapper #39

Closed
travissanderson-wf opened this issue Mar 23, 2015 · 19 comments · Fixed by #1127
Closed

Support a default HTML wrapper #39

travissanderson-wf opened this issue Mar 23, 2015 · 19 comments · Fixed by #1127
Labels
config file type-enhancement A request for a change that isn't a bug

Comments

@travissanderson-wf
Copy link

For our use case we want to be able to provide a default HTML file instead of one for every test file. See have this PR into test_runner.dart googlearchive/test_runner.dart#35 for how we are accomplishing that there.

@nex3 nex3 added the type-enhancement A request for a change that isn't a bug label Mar 23, 2015
@natebosch
Copy link
Member

We have default HTML files which are automatically provided when the test does not have it's own.

@travissanderson-wf
Copy link
Author

The purpose of this request was for us to be able to provide a customized default HTML file besides the auto-generated one that the runner creates. Is that capability available now?

@natebosch
Copy link
Member

Ah I see, no that isn't an option today. Will leave this open for consideration.

@natebosch natebosch reopened this Jun 13, 2018
@travissanderson-wf
Copy link
Author

👍

@natebosch
Copy link
Member

I think what this would take is adding some support for configuration in dart_test.yaml to point to a template to use instead of the default one provided by the test runner. We won't have bandwidth to implement this any time soon but we'd be happy to review a pull request adding this.

@wytrych
Copy link
Contributor

wytrych commented Dec 11, 2019

@natebosch are you still open for a PR on this feature? I'd be happy to look into this.

@jakemac53
Copy link
Contributor

jakemac53 commented Dec 11, 2019

Another option here would be to do this with a builder - it would be pretty trivial but would require the use of build_runner. However that is already a dependency for almost any web project.

That way the test package doesn't have to own this logic (or add the deps required to support it), and it would be easier for people to iterate? It would also allow for multiple templating languages etc.

EDIT: If there is already a basic templating language implemented (and that is how the default html file works) then doing something in this package is likely reasonable.

@wytrych
Copy link
Contributor

wytrych commented Dec 11, 2019

Using build_runner is doing something very different though, it tries to build the whole package first, not only pulling in the necessary dependencies. In a big project that is a big difference.

@jakemac53
Copy link
Contributor

jakemac53 commented Dec 11, 2019

You can't run the tests (in debug mode at least) until the whole package is built anyways - so I think for most users this wouldn't matter.

Note that it does also now have the ability to only build a specific glob of files via the --build-filter option.

@wytrych
Copy link
Contributor

wytrych commented Dec 11, 2019

Hmm when I run a single test it only builds the files that are actually imported from that test (and their dependency graph ofc)

@jakemac53
Copy link
Contributor

If you run pub run test -p chrome then yes it does that - but it compiles with dart2js which isn't easy to debug. It isn't really something we recommend people to do.

Also, as I mentioned you can now do the same with build_runner (only compile a single test). We need better documentation around this to be sure but its possible to do.

@jakemac53
Copy link
Contributor

I talked with @natebosch a bit about this. We would be open to taking a PR for this in the test package.

We would however like to ensure that it is very limited in scope. Most likely this means only supporting a single special placeholder in the template, which is for test file name. This would be string replaced to create the resulting html file.

This will ensure we don't need to add any additional dependencies, and the maintenance burden would be very low.

@wytrych
Copy link
Contributor

wytrych commented Dec 11, 2019

Ok, sounds good, I'll see what I can do.

@wytrych
Copy link
Contributor

wytrych commented Dec 12, 2019

@jakemac53 I got something working, but I'm wondering what is the best way to create the resulting html file? Is just just File().writeAsStringSync or is there some way to create a virtual file somehow?

@jakemac53
Copy link
Contributor

It should be possible to never write this file to disk at all but simply add an extra handler to the test server which knows how to serve it when requested. I believe that is how the existing default html files are done.

You can look at the browser platform code probably to find this - I can dig a bit and see if I can find a good link for you.

@jakemac53
Copy link
Contributor

jakemac53 commented Dec 12, 2019

shelf.Response _wrapperHandler(shelf.Request request) {

@wytrych
Copy link
Contributor

wytrych commented Dec 12, 2019

Thanks @jakemac53 that's interesting, I was actually looking at

p.toUri(p.withoutExtension(p.relative(path, from: _root)) + '.html'));
since this seems to be the place that is tested in
test('on Chrome', () async {

When are these different codepaths taken?

@jakemac53
Copy link
Contributor

I believe that what I linked is always what is used unless running in pub-serve mode which isn't really a supported mode at all any more (basically it allows you to run tests against a running server).

That part you linked is just where it is assigning the URL from which the suite should be loaded - but it doesn't actually generate the html. That is done in the handler, when the files are requested (assuming no explicit custom html file).

@wytrych
Copy link
Contributor

wytrych commented Dec 13, 2019

Ah that makes sense. I missed the point that the handler will get called only when there isn't an actual file found. Thanks!

jakemac53 pushed a commit that referenced this issue Jan 6, 2020
This allows for reusing one template file across all tests in use cases
where external scripts or html elements are required for all tests. The
possibility to still use local html files per test file is retained.

Fixes #39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config file type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants