search

web-platform-tests / rfcs

web-platform-tests RFCs
75 stars 63 forks source link

Readme to consider adding the 'Editor' repo to Web-Platform-Tests #24

Closed thejohnjansen closed 3 years ago

thejohnjansen commented 4 years ago

Rendered

jgraham commented 4 years ago

This would benefit from more details about what the thing actually is, including a link to the code and if possible to a live instance.

thejohnjansen commented 4 years ago

@jgraham , you are very fast. I'm on a thread with @foolip and will update. I did include a link to the repo now, but it looks like I accidentally actually added the editor repo, when I meant to just submit a PR for it. I'm following up.

foolip commented 4 years ago

The repo is at https://github.com/web-platform-tests/editor. That was an accident, but let's just leave it there and not touch it while we decide about how/if to maintain it here.

foolip commented 4 years ago

The README provides some more detail on what this is. I know that @tabatkins thinks wptest.center is rad and has a lot of potential, and @lukebjerring has also been interested in integrating it more with WPT somehow. Looks like this is our chance!

My questions:

jgraham commented 4 years ago

I have to admit I'm struggling to see the value proposition of this tool, so I'm hoping that someone can help me understand.

In principle the idea of a development environment specifically optimised for test writing seems quite reasonable. There are features that you want in that environment that aren't commonly found in editors out of the box e.g.

When I look at wpttest.center I see a three pane editor like codepen. That seems OK for tests that fit into the single script / single css file / single HTML document model, although not all tests are like that. Obviously it's familiar to many frontend developers, which is good. I see some kind of reduced devtools, although it's very unclear to me what the watchpoints thing does. I feel like this is perhaps supposed to be the central feature of this tool, so maybe understanding that would help. It looks like the generated HTML contains a testharness link by default, but I couldn't immediately see how to create non-testharness tests. I assume when you actually save the test it replaces the internal paths with the correct ones for submission?

Overall, from looking at the demo, I'm not sure it addresses the wpt-specific pain points that I feel when writing tests. I understand this may be a "less space than a nomad" type comment, but if we are going to adopt this tool I'd like to understand why we think it's going to be compelling for authors and boost the number and/or quality of test submissions we get.

(fwiw I also agree with the technical concerns that @foolip raised; this seems like a subastantial piece of code with a very different tech stack to anything else we maintain).

tabatkins commented 4 years ago

Overall, from looking at the demo, I'm not sure it addresses the wpt-specific pain points that I feel when writing tests.

It addresses all of my wpt-specific pain points. For various reasons, I find contributing to WPT by hand to be quite annoying and tedious, and it makes me instinctively avoid doing so; if someone of my reasonable level of git-skill feels this way, I can't imagine how frustrating it is for someone who uses it more casually.

Whereas using this tool, I can write a good test in moments with absolutely minimal pain. There are a few small features I want, like templates (so I don't have to copy/paste form a previous test the meta and link that CSSWG tests expect), but overall this makes test authoring easy and fun for me.

Templates for boilerplate e.g. the testharness.js scripts or reftest prelude

It already adds the testharness/etc automatically. No support for reftests yet on it, so of course it doesn't add the reftest prelude. I haven't had to use any of the complicated testharness variations, so I dunno if it works with them.

Support for running the test (notably for reftests where telling if the test actually passes as written is non-trivial).

Already there for the test structures it supports.

Integration of server-side functionality e.g. expanding out template strings, running any.js tests in each global environment.

Haven't run into the need for any of these yet. (And don't know what they are, frankly.)

Ability to distribute the test to multiple browsers in order to check that things work as expected in each of them.

Pretty trivial to do by hand - you just open the editor in the other browser and copy-paste the three panes. (If you're even using all three; most of the time I'm only using the JS and/or CSS panes.)

devtools support to make sure you're testing what you think you are.

You can console.log whatever you want; it shows up in both the Console tab and in your devtools like normal. Were you thinking of something more elaborate?

Integrated linting to ensure that the test isn't going to be rejected for trivial reasons.

Definitely a nice-to-have. So far I haven't found the linting too painful, just mildly annoying. (My main annoyance is that the editor defaults to indenting with tabs, and our linting requires spaces, so I always have to remember to manually fix that. Should be an easy change to make.)

Hand holding to create a final PR for the test.

Already does it for you - that's the "Export" option up top.

(fwiw I also agree with the technical concerns that @foolip raised; this seems like a subastantial piece of code with a very different tech stack to anything else we maintain).

Agreed, but when weighing "actually existing test-authoring tool, albeit with a weird tech stack" vs "nothing at all, and no plans to make anything", I'm gonna go with the one that actually exists (and that I already use and enjoy).

jgraham commented 4 years ago

It addresses all of my wpt-specific pain points.

OK, but can you explain what those are and what the tool is bringing? Is it literally just codepen+ability to create a PR, or something else?

tabatkins commented 4 years ago

Codepen-ish UI, automatic PR with no git-wrangling (the "clone, keep your clone up to date, branch for new things, push branch, then go to the web interface and click the make-PR button" dance is really quite bad), and inline test results at the press of a button (no need to navigate to your test in a separate browser tab).

jgraham commented 4 years ago

OK, thanks.

I think my opinion here is that if people are prepared to maintain this, and we can sort out the issues that @foolip mentioned around the stack and the different contributing requirements, then hosting this here is essentially harmless. However if it falls out of maintainance and/or fails to attract users I will be equally inclined to drop it at the first sign that it's causing any burden on the wider project.

And fwiw, based on what @tabatkins said, it seems like there are some much smaller features we could write that would have a lot of the benefits here e.g. a create-pr command that fetches the remote, rebases the local commits onto master, pushes to GH, and creates a PR from the branch automatically. GitHub's hub tool does a bunch of that stuff already so it might even be possible to just depend on that directly rather than writing the API parts from scratch. That wouldn't cover all the uses of this tool, but might help people who don't want to use a bespoke editor, but do find the GitHub admin tedious.

jugglinmike commented 4 years ago

We may be talking past each other since it seems like there are different expectations about who this can/should serve:

Without a shared understanding of the target audience, it's hard to agree on the usefulness of the tool. Even its completeness is hard to pin down: does it need reftest support? Does it need to understand templating or multi-global tests? It would be great if we had answers to those questions before taking it on and even better if we had some expectation of if/when new features would be added.

@thejohnjansen do you have access to the research that went in to the design?

@lukebjerring /@foolip do you think supplementing that research and development would be in scope for the Ecosystem Infra team?

foolip commented 4 years ago

@jugglinmike In principle a tool like this is in scope for Ecosystem Infra, and @tabatkins is a team member, so I defer to him and @thejohnjansen to set the direction here. There's probably low-hanging fruit improvements to tackle initially.

John, Tab, what do you see as the core functionality you want this tool to support?

My unprompted wish list for a tool like this:

That's somewhat at odds with the 3-pane UI, but doesn't seem like a huge stretch from where the tool is at.

gsnedders commented 4 years ago

I think my opinion here is that if people are prepared to maintain this, and we can sort out the issues that @foolip mentioned around the stack and the different contributing requirements, then hosting this here is essentially harmless. However if it falls out of maintainance and/or fails to attract users I will be equally inclined to drop it at the first sign that it's causing any burden on the wider project.

👍

foolip commented 4 years ago

@thejohnjansen do you have an update on the questions raised here, and are you blocked on anything else? In particular, do we need to get a CoC for WPT before this can proceed?

thejohnjansen commented 4 years ago

Thanks for the bump, @foolip . I have a branch with the changes, but I have not been able to figure out how the deploy works yet. I wasn't going to submit a pull request for this CL until after I got that working. I pinged @FremyCompany but haven't heard back yet. I believe the other questions can be dealt with as issues after that. The CoC as well, though I do think we should write one in parallel to this.

FremyCompany commented 4 years ago

@thejonhjansen Hey John! Not sure I actually got your ping, I'm just reading this thread for the first time. What was your question about?

I'd be happy to keep contributing but didn't have access to the Microsoft repository anymore, but if there are features I can help with, I'd definitely consider that.

thejohnjansen commented 4 years ago

It looks like I need to chat with Anton on my team about the deployment. Sorry for the interrupt @FremyCompany ...

thejohnjansen commented 4 years ago

OK. I have finally modified the Azure scripts so the website (wptest.center) is based on this new repo instead of the old one. I think we can close this rfc and address each concern individually as issues. I've already got a change locally for the strings to be modified from the old name to "editor" as well as anything that referenced Microsoft directly. I'll create that PR as soon as this is closed.

FremyCompany commented 4 years ago

should we try to do a quick meeting during tpac to discuss this?

Hexcles commented 4 years ago

@FremyCompany that's a good idea. Would you be able to drop by on Monday or Tuesday?

thejohnjansen commented 4 years ago

Sounds like a great idea. I'll be there all week.

foolip commented 4 years ago

Alright, let's park this until TPAC.

FremyCompany commented 4 years ago

In theory I'll be in the csswg on Monday and Tuesday but if we decide on a breakout session I can and would make it. Preferably on Tuesday if I might chose. But we can also get a plenary day spot, too.

foolip commented 4 years ago

Discussed as TPAC today. @thejohnjansen will share a link to the minutes here.

Conclusion is that this should block on only two things: updating the CoC and removing the CLA.

The URL will be https://editor.web-platform-tests.org to begin with.

thejohnjansen commented 4 years ago

Minutes: https://www.w3.org/2019/09/17-testing-minutes.html#item02

Issues: https://github.com/web-platform-tests/editor/issues/8 (blocking) https://github.com/web-platform-tests/editor/issues/7 (blocking) https://github.com/web-platform-tests/editor/issues/9 (Not Blocking) https://github.com/web-platform-tests/editor/issues/4 (Not Blocking)

foolip commented 4 years ago

@thejohnjansen the blocking issues have been resolved, has this been deployed or how should we proceed with this?

thejohnjansen commented 4 years ago

My understanding is that @jgraham just needs to fixup the server to point to the right location and then we can close this.

jgraham commented 4 years ago

You'll need to remind me what exactly you want here, and provide the relevant details for the DNS (i.e. the ip address if we're configuring an A record).

stephenmcgruer commented 4 years ago

We're now 4 months past the last update to this RFC. I have some questions:

tabatkins commented 4 years ago

The tool was broken some time ago for me, so I haven't been able to use it.

Yes, I can commit to maintenance for it once it gets woring.

stephenmcgruer commented 4 years ago

The tool was broken some time ago for me, so I haven't been able to use it.

I think there may be a disconnect in expectations here, as taking the tool from broken to working is the sort of task I would consider maintenance.

Can I ask what your expectations would be for maintaining the tool?

tabatkins commented 4 years ago

@thejohnjansen was the one moving this over and setting things up; it looks like it just dropped off his attention before it was finished.

I could theoretically pick that up, but it would be a significant undertaking to bring up from zero for me.

stephenmcgruer commented 4 years ago

Ack, if it never got up and running after the move then that's understandable to me.

So I guess it's down to @thejohnjansen - I think I would want to see the site as-is working before this RFC merged (and before the associated work was done to make https://editor.web-platform-tests.org point to it).

thejohnjansen commented 4 years ago

Yes, I have a task in my backlog to take care of this, but it just keeps falling to the back of the list. I need to correct that. The person who originally wrote the site no longer works at Microsoft and the person who took ownership of it after that left my team. I know that sounds like I'm a horrible manager who chases people away. Anyway, I will work on this this iteration.

thejohnjansen commented 4 years ago

OK. I believe that all of this work is now complete. We have pointed the Azure instance to the Editor repo here and I've confirmed that changes to the repo are visible on the site at https://wptest.center

@jgraham @foolip ptal

FremyCompany commented 4 years ago

Oh, thanks @thejohnjansen!

jgraham commented 4 years ago

If we're going to set this up under web-platform-tests.org then someone needs to send me the relevant DNS information :)

thejohnjansen commented 4 years ago

@jgraham I just sent you email with the information.

stephenmcgruer commented 4 years ago

Feels like we're so close here... http://editor.web-platform-tests.org/ now points to a 404, and https://editor.web-platform-tests.org/ complains about an invalid cert (which is interesting, since https://editor.center/ works).

@thejohnjansen are you aware of the 404? Do we have a plan to get this fixed?

thejohnjansen commented 4 years ago

I believe that the 404 is simply because James has not had time to update the DNS information.

jgraham commented 4 years ago

I did update the DNS, so it's now pointing at Azure, but apparently not at the right site. See e.g. https://dnsquery.org/dnstraversal/editor.web-platform-tests.org/ANY for the current settings. In particular:

editor.web-platform-tests.org. 1800 IN TXT "9609372FF37871E85ADED61E29CE34222D895A15D471B47A37890E23176A855F"
editor.web-platform-tests.org. 1800 IN A 52.173.249.137
thejohnjansen commented 4 years ago

@jgraham , my understanding is that instead of the domain, you should just use the string asuid and that the value should not be in quotes. When we go into azure to put in the domain, we get an error that we cannot find the asuid.

thejohnjansen commented 3 years ago

Checking in to see if you were able to make these changes, @jgraham. Of course, the guy on my team who knows how to do this just left the company, so I'm going to have to play around with stuff and read documentation, but I want to make sure everything is correct on your end before I break I mean fix things on our end.

gsnedders commented 3 years ago

@jgraham ping

jgraham commented 3 years ago

I think the quotes are part of the requirements for a TXT record, but I've just updated to "asuid=<long string>", so let's see if that helps.

jgraham commented 3 years ago

Oh wait, the Azure docs suggest it wants asuid.editor.web-platform-tests.org to have the TXT record? I guess I can try that then.

thejohnjansen commented 3 years ago

Wow. I actually got this working. Today I'm going to update the readme on github to point to the correct URL, etc.

you can go to https://editor.web-platform-tests.org to see it working. @tabatkins FYI.

thejohnjansen commented 3 years ago

I think this can now be closed, but I don't want to do that without LGTM from @foolip or @stephenmcgruer can one of you please take a look?

stephenmcgruer commented 3 years ago

Thanks @thejohnjansen !