Setup server side unit tests #617

youknowriad · 2017-05-03T11:55:22Z

No description provided.

youknowriad · 2017-05-03T11:56:30Z

@nylen I preferred to setup the unit tests in a separate PR. Any help here is apreciated, since it's the first time I do this.

nylen · 2017-05-04T07:08:46Z

.travis.yml

+    - php: 5.6
+      env: WP_TRAVISCI=phpcs
+    - php: 5.3
+      env: WP_VERSION=latest


See https://core.trac.wordpress.org/ticket/40407 - I don't think we need such a large matrix here. We should include:

PHP 5.2 (minimum supported WP version)

PHP 5.6 (last 5.x version)

PHP 7.1 (latest version)

nylen · 2017-05-04T07:11:36Z

.travis.yml

+    if [[ "$WP_TRAVISCI" == "phpcs" ]] ; then
+      phpcs --standard=phpcs.ruleset.xml $(find . -name '*.php')
+    fi
+  - npm run ci


Rather than coming at the end of each php job, npm run ci should be a separate job in the matrix.

nylen · 2017-05-04T07:12:09Z

tests/bootstrap.php

+/**
+ * PHPUnit bootstrap file
+ *
+ * @package Guttenberg


Typo: Guttenberg → Gutenberg

nylen · 2017-05-04T07:13:54Z

phpunit.xml.dist

+	>
+	<testsuites>
+		<testsuite>
+			<directory prefix="test-" suffix=".php">./tests/</directory>


Since we have both JS and PHP tests, I think we should call this directory something other than tests. WP core uses tests/phpunit/ and tests/qunit/, but our JS tests are inline with the files being tested. Maybe we just call ours `phpunit/ instead?

nylen · 2017-05-04T07:15:24Z

index.php

@@ -99,7 +114,7 @@ function do_blocks( $content ) {
 	global $registered_blocks;

 	// Extract the blocks from the post content
-	$open_matcher = '/<!--\s*wp:([a-z](?:[a-z0-9\/]+)*)\s+((?:(?!-->).)*)-->.*<!--\s*\/wp:\g1\s+-->/';
+	$open_matcher = '/<!--\s*wp:([a-z](?:[a-z0-9\/]+)*)\s+((?:(?!-->).)*)--><!--\s*\/wp:\g1\s+-->/';


We need the .* here, don't we? Or is this intended to match only blocks with no content?

I was thinking we should match only the empty blocks. see #586 (comment)

But with the HTML fallback feature, we need to capture the .* .

Reintroducing the .* breaks the unit tests because this

'' . 'between' . '' .

match only one block, it takes the first and the last opening/closing tags only in consideration and all what's in between is catched by the .*. I'll take a deeper look and see if I can update the regex to change the priorities.

A non-greedy match .*? should work here. Are there any cases we need to support that don't have tests?

Are there any cases we need to support that don't have tests?

We're covering everything for now I think. Oh! maybe the register the same block twice.

ntwb

I've made some requests for .travis.yml

ntwb · 2017-05-04T12:26:47Z

.travis.yml

+
+language:
+  - php
+  - node_js


Only a single language is supported by Travis CI, keep - php and remove - node_js

ntwb · 2017-05-04T12:29:07Z

.travis.yml

+      env: WP_VERSION=latest
+    - php: 5.6
+      env: TRAVISCI=phpcs
+    - env: TRAVISCI=js


I think this is meant to include a PHP version for this matrix job:

- php: 7.1 env: TRAVISCI =js

no it's not, in this case we want to run the JS job only

You still need a "language" to form the base of the build job in the matrix I believe, each job in the matrix should have both a language for the job i.e. - php: 7.1 and its associated environment matrix -env: TRAVISCI =js

If Travis were running jobs against this PR we could take a look 😖

ntwb · 2017-05-04T12:34:55Z

Why isn't Travis CI running build jobs against this PR??? 🤔

youknowriad · 2017-05-04T12:53:03Z

Why isn't Travis CI running build jobs against this PR???

@ntwb maybe because This PR is not targeting master.

I made some changes to the travis.yml. Does it look ok for you? I guess we'll check on the other PR once this is merged.

ntwb · 2017-05-04T12:59:49Z

.travis.yml

+branches:
+  only:
+    - master
+


The above 3 lines will need to be removed won't they if we want Travis to run against each PR?

ntwb · 2017-05-04T13:01:02Z

.travis.yml

+      env: WP_VERSION=latest
+    - php: 5.6
+      env: TRAVISCI=phpcs
+    - php: 5.6


Let's use the latest PHP 7.1 for this (WP Core uses the latest for its JS only job)

ntwb · 2017-05-04T13:08:28Z

.travis.yml

+    - $HOME/.composer/cache
+
+env:
+  - TRAVIS_NODE_VERSION="6.10.0"


No need for the above two lines (see next comment)

ntwb · 2017-05-04T13:09:16Z

.travis.yml

+env:
+  - TRAVIS_NODE_VERSION="6.10.0"
+
+install:


Change this to 'before_install'

ntwb · 2017-05-04T13:10:23Z

.travis.yml

+  - TRAVIS_NODE_VERSION="6.10.0"
+
+install:
+  - rm -rf ~/.nvm && git clone https://github.com/creationix/nvm.git ~/.nvm && (cd ~/.nvm && git checkout `git describe --abbrev=0 --tags`) && source ~/.nvm/nvm.sh && nvm install $TRAVIS_NODE_VERSION


Change this to '- nvm use 6' for the latest LTS NodeJS 6.x branch release

ntwb · 2017-05-04T13:14:58Z

@youknowriad See the last 2 comments above on switching to before_install and nvm

(PR reviews on mobile are frustrating)

youknowriad · 2017-05-04T13:15:32Z

@ntwb Yep I'm trying to use the .nvmrc from the repo instead

youknowriad · 2017-05-04T13:18:31Z

@ntwb oh you mean, nvm is already installed on the php travis machines. Updating 👍

ntwb · 2017-05-04T13:21:53Z

There's still no need to install nvm, Travis has full support for nvm these days

I'd suggest removing the .nvmrc file, its more than likely not needed, we can and should declare the required node and NPR versions in package.json instead

ntwb · 2017-05-04T13:29:21Z

Ok, now Travis is running 👌

@youknowriad Take a close look at each of the jobs, there are two showing "green" that they passed, this is false, there's lots of errors on those jobs.

It's nearly midnight here, so I'm off until morning

youknowriad · 2017-05-04T14:30:05Z

🎉 Tests are passing. Any other concern here?

nylen · 2017-05-04T14:56:59Z

I'd suggest removing the .nvmrc file, its more than likely not needed, we can and should declare the required node and NPR versions in package.json instead

Disagree, pretty much our whole team uses nvm during development. I for example have my shell set up to automatically switch node versions for any project that has a .nvmrc file.

youknowriad · 2017-05-04T15:32:35Z

@nylen using our current .nvmrc is not possible because of this #414. We could fix this in the said issue.

nylen · 2017-05-05T12:22:28Z

using our current .nvmrc is not possible because of this #414. We could fix this in the said issue.

Yes, I just mean that we shouldn't abandon .nvmrc.

* Setup server side unit tests

Setup server side unit tests

76654fc

youknowriad added the [Type] Build Tooling Issues or PRs related to build tooling label May 3, 2017

youknowriad self-assigned this May 3, 2017

nylen mentioned this pull request May 3, 2017

Blocks: Propose a server side rendering implementation #586

Merged

nylen reviewed May 4, 2017

View reviewed changes

youknowriad added 3 commits May 4, 2017 13:10

Testing: Fix the travis Matrix

1d7b3ef

Testing: Move the unit tests to phpunit folder

ee13005

Blocks: Non-greedy match for the server side rendering

7aec8ba

ntwb requested changes May 4, 2017

View reviewed changes

Try to fix travis config

07c793d

ntwb reviewed May 4, 2017

View reviewed changes

youknowriad force-pushed the add/server-unit-tests branch 2 times, most recently from 1f1ec98 to 1afeaa3 Compare May 4, 2017 13:10

ntwb requested changes May 4, 2017

View reviewed changes

youknowriad force-pushed the add/server-unit-tests branch from 1afeaa3 to b39cce6 Compare May 4, 2017 13:14

youknowriad force-pushed the add/server-unit-tests branch from b39cce6 to c68c629 Compare May 4, 2017 13:17

youknowriad force-pushed the add/server-unit-tests branch from c68c629 to 967a949 Compare May 4, 2017 13:19

Testing: run tests on all branches

a60d52e

youknowriad force-pushed the add/server-unit-tests branch from 967a949 to a60d52e Compare May 4, 2017 13:21

Fix JS build

378ee1b

youknowriad force-pushed the add/server-unit-tests branch from 61784b8 to 378ee1b Compare May 4, 2017 13:31

Testing Fix phpcs job

f0cd860

youknowriad force-pushed the add/server-unit-tests branch from 15c7059 to f0cd860 Compare May 4, 2017 14:12

Testing: Fix phpunit for old PHP verions

d0eb6ad

youknowriad merged this pull request into try/dynamic-blocks May 5, 2017

youknowriad deleted the add/server-unit-tests branch May 5, 2017 08:53

nylen pushed a commit that referenced this pull request May 5, 2017

Setup server side unit tests (#617)

96d6b3b

* Setup server side unit tests

youknowriad added a commit that referenced this pull request May 10, 2017

Setup server side unit tests (#617)

d5cf3d8

* Setup server side unit tests

aduth mentioned this pull request Nov 14, 2019

Coding standards: Use WordPress-Extra ruleset to prevent potential security issues #18502

Open

snyk-bot mentioned this pull request Apr 25, 2023

[Snyk] Fix for 1 vulnerabilities yassine4999/gutenberg#11

Open

npneeraj mentioned this pull request Apr 25, 2023

[Snyk] Fix for 1 vulnerabilities npneeraj/gutenberg#13

Open

This was referenced Apr 25, 2023

[Snyk] Fix for 1 vulnerabilities naiba4/gutenberg#7

Open

[Snyk] Fix for 1 vulnerabilities MaxMood96/gutenberg#95

Open

soloinovator mentioned this pull request Apr 25, 2023

[Snyk] Fix for 1 vulnerabilities soloinovator/gutenberg#84

Open

MaxMood96 mentioned this pull request Apr 25, 2023

[Snyk] Fix for 1 vulnerabilities MaxMood96/gutenberg#96

Open

snyk-bot mentioned this pull request Apr 25, 2023

[Snyk] Fix for 1 vulnerabilities captainbarber99/gutenberg#11

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup server side unit tests #617

Setup server side unit tests #617

youknowriad commented May 3, 2017

youknowriad commented May 3, 2017

nylen May 4, 2017

nylen May 4, 2017

nylen May 4, 2017

nylen May 4, 2017

nylen May 4, 2017

youknowriad May 4, 2017

nylen May 4, 2017

youknowriad May 4, 2017 •

edited

Loading

ntwb left a comment

ntwb May 4, 2017

ntwb May 4, 2017 •

edited

Loading

youknowriad May 4, 2017

ntwb May 4, 2017

ntwb commented May 4, 2017

youknowriad commented May 4, 2017

ntwb May 4, 2017

ntwb May 4, 2017

ntwb May 4, 2017

ntwb May 4, 2017

ntwb May 4, 2017

ntwb commented May 4, 2017

youknowriad commented May 4, 2017

youknowriad commented May 4, 2017

ntwb commented May 4, 2017 •

edited

Loading

ntwb commented May 4, 2017

youknowriad commented May 4, 2017

nylen commented May 4, 2017

youknowriad commented May 4, 2017

nylen commented May 5, 2017

Setup server side unit tests #617

Setup server side unit tests #617

Conversation

youknowriad commented May 3, 2017

youknowriad commented May 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad May 4, 2017 • edited Loading

Choose a reason for hiding this comment

ntwb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntwb May 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntwb commented May 4, 2017

youknowriad commented May 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntwb commented May 4, 2017

youknowriad commented May 4, 2017

youknowriad commented May 4, 2017

ntwb commented May 4, 2017 • edited Loading

ntwb commented May 4, 2017

youknowriad commented May 4, 2017

nylen commented May 4, 2017

youknowriad commented May 4, 2017

nylen commented May 5, 2017

youknowriad May 4, 2017 •

edited

Loading

ntwb May 4, 2017 •

edited

Loading

ntwb commented May 4, 2017 •

edited

Loading