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

Implement Travis CI #80

Closed
kvz opened this issue Apr 10, 2013 · 24 comments
Closed

Implement Travis CI #80

kvz opened this issue Apr 10, 2013 · 24 comments

Comments

@kvz
Copy link
Collaborator

kvz commented Apr 10, 2013

  • Do it as a nodejs / php project
  • Write a little nodejs script that parses the function headers, executes the example code in php & a separate node process, compares the results.
  • Have a way to simply disable tests. This way we can first get this system live, then start working on fixing either testcases / functions

Profit because:

  • travis is free to open source
  • we can lose a lot of ugly 2007 legacy code
  • most contributors don't setup full dev environments, so testing is not done
  • github integration. e.g. github will say if a new branch can be merged based on travis tests that are automatically ran & integrated.

This will be a big boost for php.js quality in the long-run.

Anybody who feels up to this task and make name in php.js history?

@brettz9
Copy link
Collaborator

brettz9 commented Jan 8, 2014

Congratulations! That's great work!

@kvz
Copy link
Collaborator Author

kvz commented Jan 9, 2014

Thanks Brett, this was indeed quite a big job, and we're not there yet as there are many fails now, but it's an essential first step in raising - and keeping quality at a higher level!

PS This will eventually also make it very easy to judge if merging PR requests would cause any regression as travis integrates well with github:

screen shot 2014-01-09 at 09 31 45

but obviously we first need to fix the ~80 fails before it becomes truly useful

@brettz9
Copy link
Collaborator

brettz9 commented Jan 14, 2014

It looks like a lot of the array tests may be failing because the example has a (required) dependency on the large array() function (the tests are for supporting the ordered "associative arrays" with a INI change).

@brettz9
Copy link
Collaborator

brettz9 commented Jan 14, 2014

And the "shuffle" array function is failing because the example is demonstrating random behavior.

@brettz9
Copy link
Collaborator

brettz9 commented Jan 15, 2014

"lcg_value" is also a random issue.... Though I've fixed with the header I found in cli.js, as for shuffle, but not for the arrays.

@brettz9
Copy link
Collaborator

brettz9 commented Jan 15, 2014

Sorry, I see the problem as far as arrays may be my own problem with the more recent INI-array functionality.

@brettz9
Copy link
Collaborator

brettz9 commented Jan 15, 2014

  1. strtotime ought to support timezones (which would fix its example so we could properly fix the value), but then that would require that huge timezone function.
  2. gettype is returning "object" for objects (even though the example currently says otherwise). Should we change the behavior of the function to treat all objects as arrays and thus be less useful, or do you want to keep the behavior (or add an INI to make an exception for this, though as I recall you were going to get rid of these)?
  3. I think "compact", "function_exists", "get_defined_functions", and "printf" may be failing because of a reference to window.window. Can the Node env't be made to support that?
  4. I'm perplexed by why i18n_loc_get_default is returning 'pt_PT'. Is your environment somehow setting that?

@brettz9
Copy link
Collaborator

brettz9 commented Jan 15, 2014

Finally, do you know what is causing a problem with strptime?

@brettz9
Copy link
Collaborator

brettz9 commented Jan 15, 2014

Nevermind about i18n_loc_get_default...I see the i18n_loc_set_default was probably run and one of the examples sets "pt_PT".

@brettz9
Copy link
Collaborator

brettz9 commented Jan 16, 2014

Sometimes the examples rely on default settings, but the cli is apparently remembering the old ini values from previous examples. Is there some way the environment could be re-set up after each function's examples are run (if not each example), or would that add too much of a load?

@kvz
Copy link
Collaborator Author

kvz commented Jan 16, 2014

How about we reset the phpjs object before each function is evaluated here https://github.com/kvz/phpjs/blob/master/tests/phpjsutil.js#L260?

By the way awesome work on the fixed Brett! 💖

@brettz9
Copy link
Collaborator

brettz9 commented Jan 16, 2014

Thanks :) And sure, that sounds like it ought to work. Btw, you saw my other bazillion comments above too?

@kvz
Copy link
Collaborator Author

kvz commented Jan 17, 2014

Hey Brett, Sorry I didn't get to your messages earlier! Here goes:

It looks like a lot of the array tests may be failing because the example has a (required) dependency on the large array() function (the tests are for supporting the ordered "associative arrays" with a INI change).

Fixed in 0738285

And the "shuffle" array function is failing because the example is demonstrating random behavior.

Passes now, thanks

"lcg_value" is also a random issue.... Though I've fixed with the header I found in cli.js, as for shuffle, but not for the arrays

👍

Sorry, I see the problem as far as arrays may be my own problem with the more recent INI-array functionality.

Not sure about this one

strtotime ought to support timezones (which would fix its example so we could properly fix the value), but then that would require that huge timezone function.

Not in favor of an otherwise very useful function like strtotime being dependent on shipping basically a little js-database with timezone info. imho we should make tests run in UTC, and have testcases expect that too?

gettype is returning "object" for objects (even though the example currently says otherwise). Should we change the behavior of the function to treat all objects as arrays and thus be less useful, or do you want to keep the behavior (or add an INI to make an exception for this, though as I recall you were going to get rid of these)?

I thinks as a rule of thumb, for type fetching we should stay as close to php as possible. If people want good/strict JS type fetching they should use things like underscore/lowdash. That's not something we can ever / should win nor aim for. So in short, if it's an 'associative array' we return array, cause that's what would make code work in php. It's not pretty, but it's as consistent as this project can try to be (I'm thinking of people trying to run php code in V8).

I think "compact", "function_exists", "get_defined_functions", and "printf" may be failing because of a reference to window.window. Can the Node env't be made to support that?

I already hacked window in here. It's easy to add window.window if needed. But why is it needed? Maybe I don't understand.

Let me know what you think!

PS
I've 'taken over' https://npmjs.org/package/phpjs. I'm actually thinking that whenever we're talking to the 'outside world' node.js should be our first API to talk to, vs trying to hack our way around the browser with with this.window.ActiveXObject / XMLHttpRequest / DOMParser / d.body.appendChild.

It makes more sense as php is a serverside thing and node is too. It would greatly simplify our functions (chmod has no place in the browser), and 'pure' functions such as strtotime would still be very useful in the browser. Thoughts?

@brettz9
Copy link
Collaborator

brettz9 commented Jan 18, 2014

Replies below, but first, I hope you can forgive my long reply below,
but I hope it may let you know where I'm coming from...

On 1/18/2014 7:04 AM, Kevin van Zonneveld wrote:

Hey Brett, Sorry I didn't get to your messages earlier!

No problem at all... I just wanted to make sure you noticed that there
were some. Also, btw, do you check kevin@vanzonneveld.net ?

Here goes:

It looks like a lot of the array tests may be failing because the
example has a (required) dependency on the large array() function
(the tests are for supporting the ordered "associative arrays"
with a INI change).

Fixed in 0738285

Cool, thanks.

And the "shuffle" array function is failing because the example is
demonstrating random behavior.

Passes now, thanks

"lcg_value" is also a random issue.... Though I've fixed with the
header I found in cli.js, as for shuffle, but not for the arrays

👍

Sorry, I see the problem as far as arrays may be my own problem
with the more recent INI-array functionality.

Not sure about this one

Sure, I hope to take a look when I have some time. The issue may also
just be in the examples.

strtotime ought to support timezones (which would fix its example
so we could properly fix the value), but then that would require
that huge timezone function.

Not in favor of an otherwise very useful function like |strtotime|
being dependent on shipping basically a little js-database with
timezone info. imho we should make tests run in UTC, and have
testcases expect that too?

I should have said including a dependency on the huge function, but
yeah, I understand the concern. However, I believe a number of the date
functions are relying on the locale-specific JavaScript functions rather
than UTC ones (and some may not even have a way to specify UTC
behavior). I suppose we could force users to utilize INI / setlocale() +
the timezone function to get non-UTC behavior.

gettype is returning "object" for objects (even though the example
currently says otherwise). Should we change the behavior of the
function to treat all objects as arrays and thus be less useful,
or do you want to keep the behavior (or add an INI to make an
exception for this, though as I recall you were going to get rid
of these)?

I thinks as a rule of thumb, for type fetching we should stay as close
to php as possible. If people want good/strict JS type fetching they
should use things like underscore/lowdash. That's not something we can
ever / should win nor aim for. So in short, if it's an 'associative
array' we return array, cause that's what would make code work in php.
It's not pretty, but it's as consistent as this project can try to be
(I'm thinking of people trying to run php code in V8).

Ok, so I guess I can change it.

I think "compact", "function_exists", "get_defined_functions", and
"printf" may be failing because of a reference to window.window.
Can the Node env't be made to support that?

I already hacked |window| in here
https://github.com/kvz/phpjs/blob/master/lib/phpjsutil.js#L253. It's
easy to add |window.window| if needed. But why is it needed? Maybe I
don't understand.

The use case is for references to "this.window". These references return
the window object in a normal environment, but also allow a namespaced
version to be passed a specific window for configuration. This might be
used with frames, or, as I was using it, in a Mozilla module which did
not have a "window" object. I believe our old namespace code (not sure
if you kept that with your changes to discourage large versions of
php.js) allowed for initialization with a window object.
PS

I've 'taken over' https://npmjs.org/package/phpjs. I'm actually
thinking that whenever we're talking to the 'outside world' node.js
should be our first API to talk to, vs trying to hack our way around
the browser with with |this.window.ActiveXObject| / |XMLHttpRequest| /
|DOMParser| / |d.body.appendChild|.

It makes more sense as php is a serverside thing and node is too. It
would greatly simplify our functions (|chmod| has no place in the
browser), and 'pure' functions such as |strtotime| would still be very
useful in the browser. Thoughts?

I think our approaches may diverge here due to your strong focus on the
server, and my orientation toward rich client-side applications
(privileged or not), but I very much would hope we could accommodate
both, and without leaving too big of a footprint in the code.

With better standardization in the browser, there is less of a need for
things like ActiveXObject anyways (and I would hope Node itself would
come around to support web standards like DOMParser and XMLHttpRequest
by default, even while I also hope that the browsers will support Node
APIs as they are open to do within their Addons SDK, and have already
done to a small degree with their file I/O).

While I can see your argument about PHP and Node both being server-side,
not everyone is using Node.js (no less for those many people no doubt
still using PHP who came to php.js merely for the sake of gaining a
convenient bridge to JavaScript), but mostly everyone needs to do at
least some client-side work once in a while. And some of us (like
myself) were interested in leveraging the syntax of PHP in other
(client-side) environments like browser add-ons. (Moreover, on a
practical level, there is little difference between a browser and a
server; both get privileges to their desktop; in fact, some browsers
have servers built-in (or the browser might be used to control a local
server).)

As far as the likes of chmod not having a place in the browser, believe
it or not, I actually disagree.

We can all no doubt agree that whenever developers access potentially
dangerous features like chmod, file writing, etc. on behalf of their
users, on the server or even in a client, one of course has to be careful.

But I presume you wouldn't be happy with someone working on the Node
project source saying, "Server-side developers can't be trusted to guard
their own data's safety (including creators of third-party software that
others may install to become innocent victims), so we need to prevent
developers from accessing the file system through Node (or maybe we
could allow them to access a very limited area of the file system)". You
might support server configurations or frameworks which made it harder
for developers to mess things up, but you wouldn't support the platform
restricting you, would you? Well, on second thought, maybe you wouldn't
mind in this case if you could read and write files from somewhere,
unless you were someone who wanted to be able to create a complete
server-side file browser. But I assume you can see the sometime
trade-off between security and freedom, and that you don't want security
decisions made by those higher up in the software supply chain hampering
every aspect of your freedom.

Or, to take a client-side example where one is accustomed to privileges,
would you be happy if Linux development suddenly restricted allowing
privileged executables to be installed because they could potentially
cause harm, and that instead, all newly installed software would be
restricted in what it could do on your desktop? Again, you might like
default configuration that warned you if you were going to run some
executable you had not run before, but do you really want the software
you install to be prevented from doing powerful things like accessing
the network or being able to write to local files (as say Firefox does)?

What is so inherently different between OS binary executables being
downloaded over the web and then run (whether via plugin or desktop) and
a browser downloading and executing privileged web-language-based code
with user permission--whether ActiveX, netscape enablePrivilege, or
https://github.com/brettz9/asyouwish/ (or for that matter browser
add-ons whose installation in some browsers doesn't even tell you what
privilege they will use)?

There is no difference whatsoever that I can see between binary
executables and privileged web code except in that binaries may be
theoretically more optimizable due to being created from typed languages
like C while web code is easier to create for the average developer,
more introspectable, and more convenient to users who don't need to
install a plug-in or find the file to double-click.

So, I disagree on chmod not having a place in the browser; it could be
useful as a layer on top of the add-ons SDK or
https://github.com/brettz9/asyouwish/ . If there is a use case for
building an executable program which lets you control file permissions
(and there is, as in the case of a desktop browsing replacement), there
is a use case for making this available in the browser. The browser is a
platform, and its confinement should be by user configuration (including
sensible defaults or administrator-locks) not by the language making
such arbitrary decisions.

In my opinion, I even think that if the browser did more to enhance
built-in server capabilities (e.g.,
https://addons.mozilla.org/en-US/developers/docs/sdk/latest/modules/sdk/test/httpd.html
), it would become the best place for building server functionality, so
that e.g., one's playlists built up through the downloading of music via
web application could be readily served back over the web as with other
potential use cases for users becoming publishers.

That all being said, I have to admit that for now, there is
unfortunately no cross-browser standard for doing things like chmod
(though see https://bugzilla.mozilla.org/show_bug.cgi?id=848647 ), so I
probably wouldn't be eager to add support for it yet.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 11, 2014

Apparently since a round of js-beautify changes, it appears Travis building is broken: "Error: Cannot find module 'underscore'". Any ideas what is needed to put this back on track?

@kvz
Copy link
Collaborator Author

kvz commented Feb 12, 2014

Where are you seeing this? Latest build seemed to work "fine" https://travis-ci.org/kvz/phpjs/builds/18664970 ?

@brettz9
Copy link
Collaborator

brettz9 commented Feb 12, 2014

Strange...seems ok now, thanks!

@brettz9
Copy link
Collaborator

brettz9 commented Mar 4, 2014

Can you add window.window per my comments above? If a user is working in an environment without a window, references to this.window return the window object in a normal environment, but also allow other versions like a namespaced version to provide a specific window for configuration. This might be used with frames, or, as I was using it, in a Mozilla module which did not have a "window" object.

@brettz9
Copy link
Collaborator

brettz9 commented Mar 4, 2014

Or if it makes a difference in your testing environment maybe it needs to be this.window if that will resolve differently than window.window.

kvz added a commit that referenced this issue Mar 5, 2014
@kvz
Copy link
Collaborator Author

kvz commented Mar 5, 2014

Alright, added

@brettz9
Copy link
Collaborator

brettz9 commented Mar 5, 2014

Hmm... I think the challenge is that it needs to be a circular reference...Is there a way to ensure window.window refers back to window (and that this will refer to window so that this.window refers to window.window and thus to window)?

@kvz
Copy link
Collaborator Author

kvz commented Mar 5, 2014

Ah I think get it now, sorry. How about this? 5a50fe1

Also: thanks a lot for the fixes!

@brettz9
Copy link
Collaborator

brettz9 commented Mar 5, 2014

Thanks for the commit, but even the latest one doesn't fix it. For testing, I think the easiest one to follow is function_exists. Unless there is a problem with Node not having isFinite or not having it on your window, this.window['isFinite'] should end up referring to isFinite.

@kvz
Copy link
Collaborator Author

kvz commented Mar 5, 2014

Ah you require everything to actually live inside window. In the testsuite, window is just an placeholder. That's a limitation for now unfortunately.

This works now though:

function is_finite(val) {
  window['isFinite'] = 'Brett';
  return window['isFinite'] + ' - ' + window.window['isFinite'];
$ node bin/phpjs.js --action test --name is_finite
returned:
"Brett - Brett"

But I doubt that's a big help to you then.

(~2am here, need to 💤 now)

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

No branches or pull requests

2 participants