Обсуждение: [pgadmin-hackers] [patch] Refactor: clipboard, translations, jasmine

Hi hackers,

We made some changes to start to make the javascript testable:

    - Move clipboard copying into its own file so we can test the

      interaction with it when we start adding column selection

    - Create a translations.js and translate.js which uses flask to pull in translations, but

      lets us have the flask template rendering confined to that one file,

      enabling us to test javascript files that rely on translations

    - add jasmine javascript testing which currently runs from the browser

      at /static/SpecRunner.html when the app is up.

    - delete some dead code from sqleditor.js

Why we decided to make translate.js only support interpolations of the form "%(variable)s":

   - it matches functionality in python

   - having named variables allows for multiple interpolations in one sentence without the risk that a translation will swap the order

Future things we'd like to add to this:

   - make jasmine run from the commandline with the rest of the tests

   - support for translations with number formatting

Tira & Sarah

Hi

On Fri, Feb 24, 2017 at 7:14 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> Hi hackers,
>
> We made some changes to start to make the javascript testable:
>
>     - Move clipboard copying into its own file so we can test the
>       interaction with it when we start adding column selection
>     - Create a translations.js and translate.js which uses flask to pull in
> translations, but
>       lets us have the flask template rendering confined to that one file,
>       enabling us to test javascript files that rely on translations
>     - add jasmine javascript testing which currently runs from the browser
>       at /static/SpecRunner.html when the app is up.
>     - delete some dead code from sqleditor.js
>
> Why we decided to make translate.js only support interpolations of the form
> "%(variable)s":
>    - it matches functionality in python
>    - having named variables allows for multiple interpolations in one
> sentence without the risk that a translation will swap the order

There are various issues with this patch:

- It needs to be broken up - one change, one patch. There are 4
distinct changes here, that each need to be reviewed on their own
merit.

- The changes to the translation mechanism need discussion. How will
it work? What do developers have to do differently? When will the
changes it allows be implemented?

- There are no doc updates corresponding to the changes to the
translation mechanism.

- What does the jasmine testing actually do? There are no doc or
README updates to describe it.

- There is no update to libraries.txt

- The jasmine code seems to be littered with cruft that we don't want
to be carrying in our repo. Can it be reduced to the bare minimum
CSS/JS files?

- Test code (including jasmine) needs to be isolated from the
application code, e.g. in a tests/ directory. Future versions of
pgAdmin will exclude the test suite code from their packaging.

> Future things we'd like to add to this:
>    - make jasmine run from the commandline with the rest of the tests

I think that's a pre-requisite for any additional test functionality
at this point, given that we're moving to fully automated testing.

Thanks, Dave.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Hi

On Thu, Mar 9, 2017 at 9:56 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> Hi Dave,
>
> We've split these changes up into separate patches, added docs, pulled
> jasmine out of the packaged app, and enabled running from the commandline:
>
>     Add jasmine
>     - it runs with karma from the commandline
>     - it is outside of the /web folder so that tests and test dependencies
> are excluded from the packaged app.

Actually that's not an issue - the package builds will (in most cases
now) remove the test directories, and will only include files that are
in GIT anyway. Please move everything under web/regression as right
now it's cluttering up the root directory which doesn't make sense as
it only applies to the web/ code.

>
>     Enable refactoring javascript with translations:
>     - Create a translations.js and translate.js which uses flask to pull in
> translations, but
>           lets us have the flask template rendering confined to that one
> file,
>           enabling us to test javascript files that rely on translations

OK.

>     Refactor copying text to clipboard into a separate file (this
> demonstrates use of translate.js)

I would have applied that, except that it relied on the tests :-(

>     remove some dead code from sqleditor

Thanks, I've applied that.

>     Change quotes in sqleditor to make it valid js independent of flask (it
> makes our IDE happy)
>
>> - The changes to the translation mechanism need discussion. How will
>> it work? What do developers have to do differently? When will the
>> changes it allows be implemented?
>
>
> Instead of using flask to render translations into javascript, developers
> will need to require "translate" and use it in much the same way as the
> flask _ method. So, for example, translate("My name is %(name)s.", {name:
> "Tira"}) would replace {{ _("My name is %(name)s.", {name: "Tira"}) }}

OK.

> We would like to start on these changes right away! We refactored the
> clipboard functionality out of sqleditor.js and used this translate
> functionality. The resulting file is now testable javascript.

If you can update the location of the files per my comment above, I
can get it committed. Note that the update to libraries.txt is no
longer needed, as we will neither include the additional JS modules in
the GIT repo or packages.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Mar 10, 2017 at 4:53 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> We moved package.json, karma.conf, and node_modules under web. We'll take a

This doesn't work for me:

==========
(pgadmin4)piranha:web dpage$ sudo port install nodejs7 yarn
--->  Computing dependencies for nodejs7
--->  Fetching archive for nodejs7
--->  Attempting to fetch nodejs7-7.7.2_0.darwin_16.x86_64.tbz2 from
https://packages.macports.org/nodejs7
--->  Attempting to fetch nodejs7-7.7.2_0.darwin_16.x86_64.tbz2.rmd160
from https://packages.macports.org/nodejs7
--->  Installing nodejs7 @7.7.2_0
--->  Activating nodejs7 @7.7.2_0
--->  Cleaning nodejs7
--->  Computing dependencies for yarn
--->  Fetching archive for yarn
--->  Attempting to fetch yarn-0.21.3_0.darwin_16.noarch.tbz2 from
https://packages.macports.org/yarn
--->  Attempting to fetch yarn-0.21.3_0.darwin_16.noarch.tbz2.rmd160
from https://packages.macports.org/yarn
--->  Installing yarn @0.21.3_0
--->  Activating yarn @0.21.3_0
--->  Cleaning yarn
--->  Updating database of binaries
--->  Scanning binaries for linking errors
--->  No broken files found.
--->  Some of the ports you installed have notes:
  yarn has the following notes:
    yarn stores data in:

        ~/.yarn
        ~/.yarnrc
        ~/.cache/yarn
        ~/.config/yarn

    Should you choose to install packages globally with yarn (yarn
global add), these files will not be removed if you deactivate or
uninstall yarn. By default, these packages
    will be installed in ~/.config/yarn/global, and soft-linked to
/opt/local/bin. To uninstall them all:

    $ ls -1 $HOME/.config/yarn/global/node_modules/  | xargs sudo yarn
global remove

    You may then remove the directories listed above and uninstall yarn.

    yarn is meant to replace NPM, and may cause conflicts if you use both.

    You may override the default global installation directory by
setting the PREFIX environment variable, e.g.

    mkdir -p $HOME/.config/yarn/bin # Or wherever you want
    export PREFIX=$HOME/.config/yarn/bin
    export PATH=$PREFIX:$PATH
(pgadmin4)piranha:web dpage$ yarn
yarn install v0.21.3
info No lockfile found.
[1/4] 🔍  Resolving packages...
success Nothing to install.
success Saved lockfile.
✨  Done in 0.17s.
(pgadmin4)piranha:web dpage$ sudo npm install -g karma-cli
/opt/local/bin/karma -> /opt/local/lib/node_modules/karma-cli/bin/karma
/opt/local/lib
└── karma-cli@1.0.1

(pgadmin4)piranha:web dpage$ karma start --single-run
Cannot find local Karma!
  Please install Karma by `npm install karma --save-dev`.
  If you wanna use a global instance, please set NODE_PATH env variable.
==========

I tried setting NODE_PATH to various seemingly obvious values (e.g.
/opt/local, /opt/local/lib/node_modules) but it still wouldn't work.
Google is not helping.

This also highlighted the fact that the README changes are specific to
homebrew on macOS only. At the very least they need to point out that
and that users may need to adapt to other commands on Linux/Windows or
if using MacPorts for example.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

accidentally removed the group from this thread. adding them back.

On Mon, Mar 13, 2017 at 7:08 AM, Dave Page <dpage@pgadmin.org> wrote:

On Fri, Mar 10, 2017 at 4:53 PM, Atira Odhner <aodhner@pivotal.io> wrote:
> We moved package.json, karma.conf, and node_modules under web. We'll take a

This doesn't work for me:

==========
(pgadmin4)piranha:web dpage$ sudo port install nodejs7 yarn
--->  Computing dependencies for nodejs7
--->  Fetching archive for nodejs7
--->  Attempting to fetch nodejs7-7.7.2_0.darwin_16.x86_64.tbz2 from
https://packages.macports.org/nodejs7
--->  Attempting to fetch nodejs7-7.7.2_0.darwin_16.x86_64.tbz2.rmd160
from https://packages.macports.org/nodejs7
--->  Installing nodejs7 @7.7.2_0
--->  Activating nodejs7 @7.7.2_0
--->  Cleaning nodejs7
--->  Computing dependencies for yarn
--->  Fetching archive for yarn
--->  Attempting to fetch yarn-0.21.3_0.darwin_16.noarch.tbz2 from
https://packages.macports.org/yarn
--->  Attempting to fetch yarn-0.21.3_0.darwin_16.noarch.tbz2.rmd160
from https://packages.macports.org/yarn
--->  Installing yarn @0.21.3_0
--->  Activating yarn @0.21.3_0
--->  Cleaning yarn
--->  Updating database of binaries
--->  Scanning binaries for linking errors
--->  No broken files found.
--->  Some of the ports you installed have notes:
  yarn has the following notes:
    yarn stores data in:

        ~/.yarn
        ~/.yarnrc
        ~/.cache/yarn
        ~/.config/yarn

    Should you choose to install packages globally with yarn (yarn
global add), these files will not be removed if you deactivate or
uninstall yarn. By default, these packages
    will be installed in ~/.config/yarn/global, and soft-linked to
/opt/local/bin. To uninstall them all:

    $ ls -1 $HOME/.config/yarn/global/node_modules/  | xargs sudo yarn
global remove

    You may then remove the directories listed above and uninstall yarn.

    yarn is meant to replace NPM, and may cause conflicts if you use both.

    You may override the default global installation directory by
setting the PREFIX environment variable, e.g.

    mkdir -p $HOME/.config/yarn/bin # Or wherever you want
    export PREFIX=$HOME/.config/yarn/bin
    export PATH=$PREFIX:$PATH
(pgadmin4)piranha:web dpage$ yarn
yarn install v0.21.3
info No lockfile found.
[1/4] 🔍  Resolving packages...
success Nothing to install.
success Saved lockfile.
✨  Done in 0.17s.
(pgadmin4)piranha:web dpage$ sudo npm install -g karma-cli
/opt/local/bin/karma -> /opt/local/lib/node_modules/karma-cli/bin/karma
/opt/local/lib
└── karma-cli@1.0.1

(pgadmin4)piranha:web dpage$ karma start --single-run
Cannot find local Karma!
  Please install Karma by `npm install karma --save-dev`.
  If you wanna use a global instance, please set NODE_PATH env variable.
==========

I tried setting NODE_PATH to various seemingly obvious values (e.g.
/opt/local, /opt/local/lib/node_modules) but it still wouldn't work.
Google is not helping.

This also highlighted the fact that the README changes are specific to
homebrew on macOS only. At the very least they need to point out that
and that users may need to adapt to other commands on Linux/Windows or
if using MacPorts for example.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

you could also try installing it with yarn.

On Wed, Mar 15, 2017, 9:16 AM Atira Odhner <aodhner@pivotal.io> wrote:

Hmm. I'm not sure why you can't install karma-cli. Are you able to run the tests using raw karma? (probably `web/node_modules/karma/bin/karma start` )

On Wed, Mar 15, 2017, 8:56 AM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Mar 14, 2017 at 8:16 PM, Atira Odhner <aodhner@pivotal.io> wrote:

Dave, try doing the yarn install and karma run from the web directory. karma cli expects karma to be under node_modules where it is run from. (which is where yarn will install it if you use yarn from the web directory). Karma also needs to be run from the directory where karma.conf is.

Still no joy:

piranha:pgadmin4 dpage$ cd web

piranha:web dpage$ yarn

yarn install v0.21.3

info No lockfile found.

[1/4] 🔍  Resolving packages...

success Nothing to install.

success Saved lockfile.

✨  Done in 0.28s.

piranha:web dpage$ npm install -g karma-cli

npm WARN checkPermissions Missing write access to /opt/local/lib/node_modules/karma-cli

npm WARN checkPermissions Missing write access to /opt/local/lib/node_modules

/opt/local/lib

└── karma-cli@1.0.1 

npm ERR! Darwin 16.4.0

npm ERR! argv "/opt/local/bin/node" "/opt/local/bin/npm" "install" "-g" "karma-cli"

npm ERR! node v7.7.2

npm ERR! npm  v4.0.2

npm ERR! path /opt/local/lib/node_modules/karma-cli

npm ERR! code EACCES

npm ERR! errno -13

npm ERR! syscall access

npm ERR! Error: EACCES: permission denied, access '/opt/local/lib/node_modules/karma-cli'

npm ERR!  { Error: EACCES: permission denied, access '/opt/local/lib/node_modules/karma-cli'

npm ERR!   errno: -13,

npm ERR!   code: 'EACCES',

npm ERR!   syscall: 'access',

npm ERR!   path: '/opt/local/lib/node_modules/karma-cli' }

npm ERR! 

npm ERR! Please try running this command again as root/Administrator.

npm ERR! Please include the following file with any support request:

npm ERR!     /Users/dpage/git/pgadmin4/web/npm-debug.log

piranha:web dpage$ sudo npm install -g karma-cli

Password:

/opt/local/bin/karma -> /opt/local/lib/node_modules/karma-cli/bin/karma

/opt/local/lib

└── karma-cli@1.0.1 

piranha:web dpage$ karma start --single-run

Cannot find local Karma!

  Please install Karma by `npm install karma --save-dev`.

  If you wanna use a global instance, please set NODE_PATH env variable.

 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Oops, somehow we ended up with package.json in the wrong directory! It should be under web. Here is a patch to fix it. I also updated the readme yarn/karma instructions.

Tira

On Wed, Mar 15, 2017 at 9:18 AM, Atira Odhner <aodhner@pivotal.io> wrote:

you could also try installing it with yarn.

On Wed, Mar 15, 2017, 9:16 AM Atira Odhner <aodhner@pivotal.io> wrote:

Hmm. I'm not sure why you can't install karma-cli. Are you able to run the tests using raw karma? (probably `web/node_modules/karma/bin/karma start` )

On Wed, Mar 15, 2017, 8:56 AM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Mar 14, 2017 at 8:16 PM, Atira Odhner <aodhner@pivotal.io> wrote:

Dave, try doing the yarn install and karma run from the web directory. karma cli expects karma to be under node_modules where it is run from. (which is where yarn will install it if you use yarn from the web directory). Karma also needs to be run from the directory where karma.conf is.

Still no joy:

piranha:pgadmin4 dpage$ cd web

piranha:web dpage$ yarn

yarn install v0.21.3

info No lockfile found.

[1/4] 🔍  Resolving packages...

success Nothing to install.

success Saved lockfile.

✨  Done in 0.28s.

piranha:web dpage$ npm install -g karma-cli

npm WARN checkPermissions Missing write access to /opt/local/lib/node_modules/karma-cli

npm WARN checkPermissions Missing write access to /opt/local/lib/node_modules

/opt/local/lib

└── karma-cli@1.0.1 

npm ERR! Darwin 16.4.0

npm ERR! argv "/opt/local/bin/node" "/opt/local/bin/npm" "install" "-g" "karma-cli"

npm ERR! node v7.7.2

npm ERR! npm  v4.0.2

npm ERR! path /opt/local/lib/node_modules/karma-cli

npm ERR! code EACCES

npm ERR! errno -13

npm ERR! syscall access

npm ERR! Error: EACCES: permission denied, access '/opt/local/lib/node_modules/karma-cli'

npm ERR!  { Error: EACCES: permission denied, access '/opt/local/lib/node_modules/karma-cli'

npm ERR!   errno: -13,

npm ERR!   code: 'EACCES',

npm ERR!   syscall: 'access',

npm ERR!   path: '/opt/local/lib/node_modules/karma-cli' }

npm ERR! 

npm ERR! Please try running this command again as root/Administrator.

npm ERR! Please include the following file with any support request:

npm ERR!     /Users/dpage/git/pgadmin4/web/npm-debug.log

piranha:web dpage$ sudo npm install -g karma-cli

Password:

/opt/local/bin/karma -> /opt/local/lib/node_modules/karma-cli/bin/karma

/opt/local/lib

└── karma-cli@1.0.1 

piranha:web dpage$ karma start --single-run

Cannot find local Karma!

  Please install Karma by `npm install karma --save-dev`.

  If you wanna use a global instance, please set NODE_PATH env variable.

 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

That go it! Thanks, patch applied.

On Wed, Mar 15, 2017 at 2:46 PM, Atira Odhner <aodhner@pivotal.io> wrote:

Oops, somehow we ended up with package.json in the wrong directory! It should be under web. Here is a patch to fix it. I also updated the readme yarn/karma instructions.

Tira

On Wed, Mar 15, 2017 at 9:18 AM, Atira Odhner <aodhner@pivotal.io> wrote:

you could also try installing it with yarn.

On Wed, Mar 15, 2017, 9:16 AM Atira Odhner <aodhner@pivotal.io> wrote:

Hmm. I'm not sure why you can't install karma-cli. Are you able to run the tests using raw karma? (probably `web/node_modules/karma/bin/karma start` )

On Wed, Mar 15, 2017, 8:56 AM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Tue, Mar 14, 2017 at 8:16 PM, Atira Odhner <aodhner@pivotal.io> wrote:

Dave, try doing the yarn install and karma run from the web directory. karma cli expects karma to be under node_modules where it is run from. (which is where yarn will install it if you use yarn from the web directory). Karma also needs to be run from the directory where karma.conf is.

Still no joy:

piranha:pgadmin4 dpage$ cd web

piranha:web dpage$ yarn

yarn install v0.21.3

info No lockfile found.

[1/4] 🔍  Resolving packages...

success Nothing to install.

success Saved lockfile.

✨  Done in 0.28s.

piranha:web dpage$ npm install -g karma-cli

npm WARN checkPermissions Missing write access to /opt/local/lib/node_modules/karma-cli

npm WARN checkPermissions Missing write access to /opt/local/lib/node_modules

/opt/local/lib

└── karma-cli@1.0.1 

npm ERR! Darwin 16.4.0

npm ERR! argv "/opt/local/bin/node" "/opt/local/bin/npm" "install" "-g" "karma-cli"

npm ERR! node v7.7.2

npm ERR! npm  v4.0.2

npm ERR! path /opt/local/lib/node_modules/karma-cli

npm ERR! code EACCES

npm ERR! errno -13

npm ERR! syscall access

npm ERR! Error: EACCES: permission denied, access '/opt/local/lib/node_modules/karma-cli'

npm ERR!  { Error: EACCES: permission denied, access '/opt/local/lib/node_modules/karma-cli'

npm ERR!   errno: -13,

npm ERR!   code: 'EACCES',

npm ERR!   syscall: 'access',

npm ERR!   path: '/opt/local/lib/node_modules/karma-cli' }

npm ERR! 

npm ERR! Please try running this command again as root/Administrator.

npm ERR! Please include the following file with any support request:

npm ERR!     /Users/dpage/git/pgadmin4/web/npm-debug.log

piranha:web dpage$ sudo npm install -g karma-cli

Password:

/opt/local/bin/karma -> /opt/local/lib/node_modules/karma-cli/bin/karma

/opt/local/lib

└── karma-cli@1.0.1 

piranha:web dpage$ karma start --single-run

Cannot find local Karma!

  Please install Karma by `npm install karma --save-dev`.

  If you wanna use a global instance, please set NODE_PATH env variable.

 

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--

Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company