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

Convert writable stream tests to web platform tests format #492

Closed
5 tasks done
domenic opened this issue Aug 4, 2016 · 10 comments
Closed
5 tasks done

Convert writable stream tests to web platform tests format #492

domenic opened this issue Aug 4, 2016 · 10 comments

Comments

@domenic
Copy link
Member

domenic commented Aug 4, 2016

After #488 lands we should do this


Update 2016-10-25: here is a remaining checklist of files, after #540 lands:

  • bad-strategies.js
  • bad-underlying-sinks.js (can go to existing to-upstream bad-underlying-sinks.js)
  • brand-checks.js
  • byte-length-queuing-strategy.js
  • count-queuing-strategy.js

Updated 2016-11-07: #640 is taking care of the above.

@tyoshino
Copy link
Member

tyoshino commented Aug 25, 2016

I (and @ricea also maybe) are going to work on this.

What would the best work flow? Maybe, local review somewhere, and then PR to w3c/.

Creating PR to fork would be overkilling, right?

Just commit to personal forks and then get them reviewed before upstream PR?
tyoshino/web-platform-tests@4cc6bf2

@domenic
Copy link
Member Author

domenic commented Aug 25, 2016

Oh, very exciting! I think doing something similar to what we did for readable stream makes sense: use this repo as a staging ground and convert the tests here, then when we feel they're "done", move them to WPT in one PR. The benefit there is that we can continue to validate the tests against the reference implementation as we go.

@tyoshino
Copy link
Member

OK. As we've removed the directory in 85260dc and is now used for checking out wpt from w3c repo, let me create a new directory dedicated for staging/reviewing.

@tyoshino
Copy link
Member

Hmm, before 85260dc, we had to manually copy the streams test to a full checkout of w3c wpt so that the tests can use e.g. stuff in service-workers/resources? Sorry, I forgot that.

Now, to keep the converted wpt tests for writable streams in this repository, basically we need to have a separate directory. To allow the tests in it to use the files (service-workers/resources), we need to create a symlink (not good for Windows checkout?) or (partially) clone the repo. It's not something we want to do, I think.

@tyoshino
Copy link
Member

Hmm, it's awkward, but negation entry in .gitignore would help. See #510.

@domenic
Copy link
Member Author

domenic commented Aug 30, 2016

Please leave the piping tests to me and I'll work on converting them as part of #512.

tyoshino added a commit that referenced this issue Aug 31, 2016
This commit is for the issue #492.

wpt-runner invocation is tweaked by domenic.

We'll use the to-upstream-wpts/ directory for placing converted wpt files until it's ready for upstreaming them to the w3c repo.

All the tests under the directory are run automatically when we run "npm run wpt" in addition to the ones already moved to the w3c repo.
@tyoshino
Copy link
Member

Please leave the piping tests to me and I'll work on converting them as part of #512.

Got it, domenic.

@ricea It's ready for converting more tests into wpt. If you're willing to take some of them, please share which items you're going to take here. Converted tests should be placed in the to-upstream-wpts/ directory.

@ricea
Copy link
Collaborator

ricea commented Oct 13, 2016

I am porting the tests in test/writable-stream.js this week. The tests in test/writable-stream-abort.js are available if anyone wants to pick them up.

@marvinhagemeister
Copy link
Contributor

@ricea Awesome, I'll have a go at writeable-stream-abort.js

@domenic
Copy link
Member Author

domenic commented Oct 25, 2016

I've updated the OP with a remaining checklist of files to move.

@domenic domenic closed this as completed Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants