Skip to content

add arraybuffer test file #1477

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

Merged
merged 1 commit into from
Dec 9, 2016

Conversation

jiangzidong
Copy link
Contributor

@jiangzidong jiangzidong commented Dec 8, 2016

#1475
add test files for ArrayBuffer builtin object #1467
JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]

* limitations under the License.
*/

var a = new ArrayBuffer();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after ArrayBuffer

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zherczeg There's actually no space needed after ArrayBuffer. Doing a quick "find -name '*.js'|xargs cat|grep new" in jerry-test-suite shows that there is not a single constructor invocation where there is a space before the parentheses.

@zherczeg
Copy link
Member

zherczeg commented Dec 8, 2016

This patch looks quite good. Please fix the style issues first.

@jiangzidong jiangzidong force-pushed the arraybuffertest branch 2 times, most recently from 0b1b536 to 29c63a7 Compare December 8, 2016 08:04
@LaszloLango
Copy link
Contributor

Please update the commit message. Add the related issue number to it. Ex.: "Fixes #1475"

@zherczeg
Copy link
Member

zherczeg commented Dec 8, 2016

@tilmannOSG hm, you are rgiht. Example 08.01-001.js:

assert(typeof (a) === "undefined");

There is a space after typeof, but there is no space after assert. This is kind of a mixed style, but it is not a good practice.

What shall we do? Let people use whatever they want? These are tests, so perhaps this way we would cover a bigger range of issues.

jiangzidong added a commit to jiangzidong/jerryscript that referenced this pull request Dec 8, 2016
fix issue jerryscript-project#1475
JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]
@tilmannOSG
Copy link

tilmannOSG commented Dec 8, 2016

@zherczeg I think for now people should just follow whichever style is the most common, e.g. in this case keep the space after the typeof.
In the long-term we can consider adopting one of the common JavaScript coding standards.
I think lexer stress tests should be separate (rather than variations in formatting style), having all the JavaScript tests written in a consistent manner still provides a lot of value :)

@zherczeg
Copy link
Member

zherczeg commented Dec 8, 2016

LGTM

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@tilmannOSG tilmannOSG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reformat all function/constructor calls to not include a space before the opening parenthesis (this is the formatting used for all the JavaScript source files in JerryScript right now).

* limitations under the License.
*/

var a = new ArrayBuffer ();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No space between ArrayBuffer and the parentheses please (as discussed previously, seems GitHub lost that feedback).

*/

var a = new ArrayBuffer ();
assert (typeof a === 'object');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: No space between assert and the opening parenthesis please.

fix issue jerryscript-project#1475
JerryScript-DCO-1.0-Signed-off-by: Zidong Jiang [email protected]
@tilmannOSG tilmannOSG merged commit fb2edd8 into jerryscript-project:master Dec 9, 2016
@jiangzidong jiangzidong deleted the arraybuffertest branch December 9, 2016 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants