Edit & run examples from unittests #1297

wilzbach · 2016-05-13T12:27:29Z

Hey all,

I wanted to be able to execute the examples directly from the documentation for a long time. It turns out, it's not that hard ;-)

The current module and write,writeln are imported. It might be that a couple of tests depend on the private imports in a module, but this is discouraged anyways as the public documented unittests should really only need the current module imported. I tested it randomly with a couple of modules and so far everything seems to work well.

This feature is experimental and I would love feedback on it.
As dpaste requires CORS and the origin must be dlang.org, you have to trick a bit to test it.
If you happen to use pdns, it as easy as pdnsd-ctl add a 127.0.0.1 dlang.org though.

Therefore here are some images as impressions:

writeln works too:

@CyberShadow: maybe we can even send the D version to DPaste so we never run into troubles of library mismatches?

Edit: maybe @CyberShadow can add DAutoTest to the origin whitelist of DPaste?

JackStouffer · 2016-05-13T14:16:23Z

css/style.css

+/* Style for the example run buttons on the Phobos library documentation */
+.d_example_buttons {
+    text-align: right;
+}


new line here please

may I ask for the motivation?

CyberShadow · 2016-05-13T14:20:06Z

Why is there so much new JS code? Can't the front page code be reused?

CyberShadow · 2016-05-13T14:21:05Z

Should this not be enabled only for Phobos pages?

CyberShadow · 2016-05-13T14:23:49Z

As I mentioned at DConf, this should only be enabled for documented unittests (runnable examples). This may need a small compiler patch to allow distinguishing the two. A lot of the non-unittest examples are nonsensical out of context (see e.g. std.file.timeLastModified).

JackStouffer · 2016-05-13T14:29:31Z

js/run_examples.js

@@ -0,0 +1,159 @@
+$(document).ready(function()
+{
+    if ($('body')[0].id == "Home")


document.getElementsByTagName('body')[0].id == "Home"

I'm all for jQuery for when it abstracts differences between browsers, but for the simple stuff let's stick with plain JS.

If jQuery is already in use then there is no reason not to use all of it.

JackStouffer · 2016-05-13T14:51:32Z

If jQuery is already in use then there is no reason not to use all of it.

@CyberShadow We really shouldn't. I know I added some code to the site that used JQuery, but I recently changed my mind on this.

Just run this code in your console on the dlang.org home and look at the performance:

function test1() {
    return document.getElementsByTagName('body')[0].id == "Home";
}
function test2() {
    return $('body').id == "Home";
}

My results

> console.time("test"); for (var i = 0; i < 100000; i++) { test1(); } console.timeEnd("test");
test: 8928.709ms
> console.time("test"); for (var i = 0; i < 100000; i++) { test2(); } console.timeEnd("test");
test: 26968.555ms

For a few extra keystrokes you suddenly get 3x faster.

CyberShadow · 2016-05-13T14:53:05Z

Last I checked we had no pages with hundreds of thousands of runnable examples on them.

Profile then optimize. What you are suggesting is premature optimization.

wilzbach · 2016-05-13T14:55:31Z

js/run_examples.js

+
+    // only for std at the moment
+    if(!$('body').hasClass("std"))
+        return;


This enables the tests only on phobos

Would be nice to avoid including the JS file at all on other pages, so there's no useless impact on load speed.

adamdruppe · 2016-05-13T14:57:01Z

On Fri, May 13, 2016 at 07:53:06AM -0700, Vladimir Panteleev wrote:

Profile then optimize. What you are suggesting is premature optimization.

If it is trivial to use the better way, you might as well just do it
right the first time....

JackStouffer · 2016-05-13T15:09:51Z

Last I checked we had no pages with hundreds of thousands of runnable examples on them.

No, we don't. But that doesn't mean that we should be consciously using code that is slower in order to save like, ten characters.

Profile then optimize. What you are suggesting is premature optimization.

These things add up.

CyberShadow · 2016-05-13T15:18:49Z

I'm going to have to ask you to kindly stop it with the bullshit.

The particular line of code you have clinged yourself to criticizing runs once per page load.
The code is pre-existing, the line has been simply copied from existing code in run.js.

If you want to make a positive contribution, you could try rewriting run.js to not require jQuery. Show some readable, maintainable code as the result; show a substantial, repeatable and practically significant improvement in performance, and we can discuss merging that.

Otherwise, I suggest you refrain from such future criticism, because the only impact you're making is aggravating contributors who are actually trying to make a positive change.

JackStouffer · 2016-05-13T15:38:04Z

Dude,

$('body')[0].id == "Home" to document.getElementsByTagName('body')[0].id == "Home"

$('pre[class~=d_code]') to document.querySelectorAll('pre[class~=d_code]')

I'm not asking for much here. I'm asking that thirty seconds be taken to use the faster version. And if you disagree with me, you're the one with merge rights at the end of the day.

CyberShadow · 2016-05-13T15:41:07Z

I see my point did not get through.

It is not at all faster, in this case. The only effect such a change would have is make the code longer and more inconsistent.

wilzbach · 2016-05-13T16:27:25Z

As I mentioned at DConf, this should only be enabled for documented unittests (runnable examples). This may need a small compiler patch to allow distinguishing the two. A lot of the non-unittest examples are nonsensical out of context (see e.g. std.file.timeLastModified).

That would be great to have. Can we just have two separate css classes maybe?

schuetzm · 2016-05-17T08:31:03Z

I agree with CyberShadow. Getting rid of jQuery altogether can be worthwhile, but only because it would be one dependency fewer, not because $() adds a quarter of a millisecond to the page load time. Especially if we already have existing code that does what you want, just reuse that instead of adding another copy and letting the copies diverge.

nazriel · 2016-05-17T18:46:21Z

js/run_examples.js

+                    + '</div>'
+                + '</div>'
+        );
+        console.log(orig);


Won't it break on IE?
IIRC IE doesn't like console. too much ;)

Won't it break on IE?
IIRC IE doesn't like console. too much ;)

Oh that was just for testing - will be removed soon. Sorry about that.

nazriel · 2016-05-17T18:52:48Z

@CyberShadow: maybe we can even send the D version to DPaste so we never run into troubles of library mismatches?

What do you mean by this?

wilzbach · 2016-05-17T18:54:08Z

What do you mean by this?

s.t. like

compiler: {
  name: "dmd",
  version: "2.071"
}

nazriel · 2016-05-17T18:57:30Z

Hm - dpaste uses the newest stable DMD for any request from dlang.org

How would you annotate specific unittest/example to run with different compiler version?
(actually I am curious - you have some clever ideas - maybe we could get rid of that MD5 madness on main page as well).

wilzbach · 2016-05-17T19:01:53Z

Hm - dpaste uses the newest stable DMD for any request from dlang.org

Oh I didn't know that. That's great!

How would you annotate specific unittest/example to run with different compiler version?
(actually I am curious - you have some clever ideas - maybe we could get rid of this MD5 madness on main page as well).

Have you tried user-defined attributes.
This is probably not useful for Phobos documentation, but feel free to ping me at your Dpaste repo, maybe we can continue this discussion there?

CyberShadow · 2016-05-17T20:12:06Z

How would you annotate specific unittest/example to run with different compiler version?

That is not right. If it's a Phobos function, then use the version from which that code comes from, no?

nazriel · 2016-05-18T09:14:19Z

That is not right. If it's a Phobos function, then use the version from which that code comes from, no?

Yeah - as I said dpaste uses newest-dmd available for dlang.org's examples.

Same goes for phobos. The only thing worth changing probably would be phobos' prerelease docs - there we probably should use nightly builds of DMD @ Dpaste.

I was asking mostly out of curiosity - I though that maybe @wilzbach has got an idea how we could annotate code examples somehow - so for example we could get rid of MD5 sums co-related with examples on the main page.

I had an idea that maybe we could use comments in code examples.
Like:

// @input: Mercury\nVenus\nEarth\nMars\nJupiter\nSaturn\nUranus\nNeptune
// @args: foo bar if needed
// Sort lines
import std.stdio, std.array, std.algorithm;

void main()
{
    stdin
        .byLineCopy
        .array
        .sort!((a, b) => a > b) // descending order
        .each!writeln;
}

Anyways - sorry for the off-topic.

Back to the case:
In overall this Pull Request is something that some time ago we were talking about with Andrei.
He wanted to have all examples in phobos documentation runnable.
Back in the day examples were heavily dependant on external modules being imported (and of course there were no import statements in examples), external input from stdin or arguments and so on.

From what I can see now mostly there are self-contained assertions and so on and the missing bits like std.base64 should be just corrected to either provide imports, stdin etc or re-arrange example entirely.

@wilzbach btw if you need anything on dpaste part - feel free to ping me. It will be open-sourced very soon but meanwhile I can help ASAP with any requirments.

MartinNowak · 2016-05-18T16:41:14Z

Great!
I think the biggest problem is that unittest examples don't currently produce output, so running them is sort of boring.

wilzbach · 2016-07-16T23:54:42Z

It's now only enabled on the prerelease pages as I assume that most users won't check those pages and we have a safe playground to test this feature?

Would this strategy work? (I am bit spare on time atm)
At least it should be easier for others to contribute ;-)

wilzbach · 2016-12-10T08:38:33Z

Okay I managed to find some time to revive this PR. As discussed above to ensure that we won't introduce regressions and verify the runnability of existing snippets, we do need to run them :)
I moved the text extraction script to dlang/tools (dlang/tools#203) and opened a PR at Phobos dlang/phobos#4943 which will enable CI checking for the public unittests.

andralex

LGTM, make sure you fix the ddoc tester

wilzbach · 2016-12-16T14:47:06Z

Okay with dlang/phobos#4943 I this is finally in a usuable state :)
Two remarks:

It's only enabled for a subset of modules (= those that are automatically checked at Phobos)
It's only enabled for the pre-release pages for now as the missing import changes need to propagate to the next release. Imho the pre-release pages don't have many visitors, so we should have a safe playground to test this

andralex · 2016-12-16T14:48:20Z

Great. About 1 - where is that list of modules for which this is enabled? And what to do to expand it?

wilzbach · 2016-12-16T15:54:27Z

Great. About 1 - where is that list of modules for which this is enabled?

There's a hard-coded exclusion list:

https://github.com/dlang/phobos/blob/master/posix.mak#L523

This PR contains a copy of it

https://github.com/dlang/dlang.org/pull/1297/files#diff-034369de775110cb179ed1e925b62a0fR53

And what to do to expand it?

Simple: remove a module from the exclusion list, have a look at the missing imports, make a PR.
It shouldn't take that long - maybe I can fit it into it into this weekend ;-)

wilzbach · 2016-12-19T05:08:22Z

Simple: remove a module from the exclusion list, have a look at the missing imports, make a PR.
It shouldn't take that long - maybe I can fit it into it into this weekend ;-)

e.g. dlang/phobos#4966

@andralex Unfortunately I can't merge stuff here. Is anything blocking this experiment?

edit: for convenience here's a preview link ("run" doesn't work here as dtest isn't allowed in the CORS settings of dpaste)

andralex · 2016-12-19T14:40:23Z

This is awesome! FWIW @CyberShadow I saw a confusing message in the log of your doc builder:

fatal: 'refs/ae-sys-d-cache/website-db6a6663b26d89fea585cb107f62262e21922c6e-c30d9ea341bb3b6ebdd1483aa91ae457' - not a valid ref

Yet the build was successful.

andralex · 2016-12-19T14:40:37Z

cool cool cool thanks @wilzbach

CyberShadow · 2016-12-19T14:41:43Z

FWIW @CyberShadow I saw a confusing message in the log of your doc builder:

Yep, those are normal. Digger checks whether a cache entry exists by asking git to describe it, and git prints that to stderr when it doesn't (i.e. on a cache miss).

andralex · 2016-12-19T16:09:10Z

@wilzbach saw the list thx, if you work on it great, if you stop at a point please put the balance of it in bugzilla. Thanks!

wilzbach · 2016-12-19T17:10:07Z

cool cool cool

Yeah it's online :)

https://dlang.org/phobos-prerelease/std_algorithm_searching.html#.minElement

if you stop at a point please put the balance of it in bugzilla. Thanks!

https://issues.dlang.org/show_bug.cgi?id=16984 (extend list of runnable modules on dlang.org)
https://issues.dlang.org/show_bug.cgi?id=16985 (enabling the unittests after 2.073)

andralex · 2016-12-19T17:41:52Z

Cool! Just tried one, it works!! How about enabling everything for ddox e.g. https://dlang.org/library-prerelease/std/algorithm/iteration/cache.html, is this easy? cc @s-ludwig

wilzbach · 2016-12-21T07:15:07Z

How about enabling everything for ddox e.g. https://dlang.org/library-prerelease/std/algorithm/iteration/cache.html, is this easy? cc @s-ludwig

Started a discussion:
dlang/ddox#137

wilzbach · 2016-12-23T19:06:14Z

Hm - dpaste uses the newest stable DMD for any request from dlang.org

@nazriel do you also maintain a copy of the latest nightly release?
This would allow to ensure that the examples on the prerelease pages are really runnable. Some of them are not as of now :/
Any way I can help you with this?

wilzbach force-pushed the run_edit_examples branch from 60b6cd1 to 0adb6a9 Compare May 13, 2016 13:02

JackStouffer reviewed May 13, 2016
View reviewed changes

wilzbach reviewed May 13, 2016
View reviewed changes

wilzbach force-pushed the run_edit_examples branch from 0adb6a9 to 8c2bdaf Compare May 15, 2016 19:55

wilzbach mentioned this pull request May 16, 2016

Implement dpaste execution driver dlang-tour/core#60

Closed

nazriel reviewed May 17, 2016
View reviewed changes

wilzbach force-pushed the run_edit_examples branch 2 times, most recently from 5b7be7c to cf8167b Compare May 25, 2016 18:08

wilzbach force-pushed the run_edit_examples branch from a3b5d47 to af2a4f1 Compare July 16, 2016 23:49

wilzbach mentioned this pull request Jul 26, 2016

fix Issue 16311 - toHash for Slice is not defined dlang/phobos#4639

Merged

wilzbach mentioned this pull request Dec 10, 2016

Add check for public unittests dlang/phobos#4943

Merged

andralex approved these changes Dec 10, 2016

View reviewed changes

wilzbach mentioned this pull request Dec 15, 2016

Fix issue 16970 - Fix deprecations and warnings when compiling Phobos dlang/phobos#4956

Merged

wilzbach force-pushed the run_edit_examples branch 3 times, most recently from d36a9c7 to 752f43d Compare December 15, 2016 22:58

wilzbach added 3 commits December 16, 2016 15:43

clean messing with the string prototype

00dae96

edit library examples directly on dlang.org

5eff226

always use SSL for dpaste

c1d18f4

wilzbach force-pushed the run_edit_examples branch from 752f43d to c1d18f4 Compare December 16, 2016 14:43

Inject run_examples for prerelease pages through EXTRA_FOOTERS

46c28fb

andralex merged commit 6063f49 into dlang:master Dec 19, 2016

wilzbach deleted the run_edit_examples branch December 19, 2016 16:52

wilzbach mentioned this pull request Dec 21, 2016

Allow runnable unittests like on dlang.org dlang/ddox#137

Closed

Uh oh!

Edit & run examples from unittests #1297

Edit & run examples from unittests #1297

Uh oh!

Conversation

wilzbach commented May 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented May 13, 2016

Uh oh!

CyberShadow commented May 13, 2016

Uh oh!

CyberShadow commented May 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JackStouffer commented May 13, 2016

Uh oh!

CyberShadow commented May 13, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamdruppe commented May 13, 2016

Uh oh!

JackStouffer commented May 13, 2016

Uh oh!

CyberShadow commented May 13, 2016

Uh oh!

JackStouffer commented May 13, 2016

Uh oh!

CyberShadow commented May 13, 2016

Uh oh!

wilzbach commented May 13, 2016

Uh oh!

schuetzm commented May 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nazriel commented May 17, 2016

Uh oh!

wilzbach commented May 17, 2016

Uh oh!

nazriel commented May 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented May 17, 2016

Uh oh!

CyberShadow commented May 17, 2016

Uh oh!

nazriel commented May 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MartinNowak commented May 18, 2016

Uh oh!

wilzbach commented Jul 16, 2016

Uh oh!

wilzbach commented Dec 10, 2016

Uh oh!

andralex left a comment

Choose a reason for hiding this comment

Uh oh!

wilzbach commented Dec 16, 2016

Uh oh!

andralex commented Dec 16, 2016

Uh oh!

wilzbach commented Dec 16, 2016

Uh oh!

wilzbach commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wilzbach commented May 13, 2016 •

edited

Loading

nazriel commented May 17, 2016 •

edited

Loading

nazriel commented May 18, 2016 •

edited

Loading

wilzbach commented Dec 19, 2016 •

edited

Loading

wilzbach commented Dec 19, 2016 •

edited

Loading