Native tests fixes. #1256
Native tests fixes. #1256
Conversation
|
@dilijev @obastemur thanks for the feedback. Please have a look at this. |
| // | ||
| fread((void*)contentsRaw, sizeof(char), lengthBytes, file); | ||
| size_t readBytes = fread((void*)contentsRaw, sizeof(char), lengthBytes, file); | ||
| if (readBytes < lengthBytes) |
There was a problem hiding this comment.
I see you took this from ch/helpers. For a big file sitting on a slow drive (i.e. a drive on network) this will be most likely failing. I'm adding these both to my list. Gonna send a PR for them. For now; this one at least fails in the right place.
|
LGTM (fread related change) |
bin/NativeTests/Scripts/splay.js
Outdated
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| // Copyright 2008 the V8 project authors. All rights reserved. | ||
| // Copyright 2013 the V8 project authors. All rights reserved. |
There was a problem hiding this comment.
Not sure if this copyright year should be updated, if in fact the version of the file you have here is from 2008. Maybe hold off on this until you can fix the test w.r.t. the version we already have in the repo?
|
👍 for adding Jenkins support. Thanks! Although this makes me wonder if we could have refactored the Jenkins stuff better... |
|
for Jenkins, when do we use the Jenkins.testall.cmd? |
|
@akroshg We don't; it's just convenient to have for starting a local test run of all flavors (and then e.g. going for lunch). I mostly used it while I was developing the Jenkins definitions. The Jenkins scripts are totally self-contained and you can run them locally, which can be helpful for tracking down issues that don't seem to repro in other builds. |
|
hmm, may be testall can call testone.cmd for each combination. |
|
ok I'll remove the splay changes and merge in. |
|
@akroshg I want to say I tried that (since it's the obvious choice) and decided against it for whatever reason. I'll look into it again at some point in the future. There's no need for all the code to be duplicated; it just needs to be refactored correctly. :) |
Added a check to return value of fread. Updated the copy-right. Added jenkins support.
5 participants
Added a check to return value of fread. Updated the copy-right. Added jenkins support.
Note : Addressing comments on the previous PR of adding native tests. Thanks for the feedback.