test: add serialization tests #237
Conversation
| ] |
There was a problem hiding this comment.
What was changed on this line?
nechaido
force-pushed
the
add-serialization-tests
branch
2 times, most recently
from
c8e2622 to
c92a465
Compare
|
I is easier to review this PR using comparison with #236 |
nechaido
force-pushed
the
add-serialization-tests
branch
from
c92a465 to
4910482
Compare
| @@ -0,0 +1,5 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
Why is this in test/fixtures/json5?
There was a problem hiding this comment.
I placed it here because they are tests for json5 specification, byt i guess I should have placed them in test/fixtures/todo/json5/
nechaido
force-pushed
the
add-serialization-tests
branch
from
9fa3821 to
300f718
Compare
| serialized: '{$:\'dollar\',_$_:\'multiple symbols\'}' | ||
| }, | ||
| { | ||
| name: 'object with unicode unquoted key', |
There was a problem hiding this comment.
"unicode" should be capitalized here.
There was a problem hiding this comment.
Also, it is better to name this test case "object with identifier name, containing non-latin Unicode literals, as a key".
| serialized: '{42:true}' | ||
| }, | ||
| { | ||
| name: 'object with unquoted keys', |
There was a problem hiding this comment.
And this one should be called "object with identifier names as keys".
There was a problem hiding this comment.
This name was taken from corresponding test for JSON5 parsing test: test/fixtures/json5/objects/unquoted-keys.json5.
There was a problem hiding this comment.
@nechaido, yes, and these JSON5 tests are imported while you are creating new tests, in my opinion, it is better to name them correctly.
|
|
||
| module.exports = [ | ||
| { | ||
| name: 'string with inline comment like substring', |
There was a problem hiding this comment.
This name is a bit misleading here.
|
|
||
| module.exports = [ | ||
| { | ||
| name: 'string with substring like inline comment', |
There was a problem hiding this comment.
This should be called "string containing inline comment".
| }, | ||
| { | ||
| name: 'object with identifier name' + | ||
| ', containing non-latin Unicode literals, as a key', |
There was a problem hiding this comment.
You shouldn't start a new line with a comma.
nechaido
force-pushed
the
add-serialization-tests
branch
from
c41a64c to
411ba26
Compare
nechaido
force-pushed
the
add-serialization-tests
branch
from
411ba26 to
9c6d914
Compare
nechaido
force-pushed
the
add-serialization-tests
branch
from
9c6d914 to
1c088f7
Compare
|
@aqrln ping. |
PR-URL: #237 Reviewed-By: Mykola Bilochub <nbelochub@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
|
Landed in 3807446. |
PR-URL: #237 Reviewed-By: Mykola Bilochub <nbelochub@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
PR-URL: #237 Reviewed-By: Mykola Bilochub <nbelochub@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
PR-URL: metarhia/jstp#237 Reviewed-By: Mykola Bilochub <nbelochub@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
PR-URL: metarhia/jstp#237 Reviewed-By: Mykola Bilochub <nbelochub@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
PR-URL: metarhia/jstp#237 Reviewed-By: Mykola Bilochub <nbelochub@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Based on #236.