add unit testing fix some bugs by peter279k · Pull Request #4 · nexylan/nexycrypt

peter279k · 2016-10-03T00:52:05Z

Change log

change the steps (1 to 3) to step (0 to 3) because of some bugs.
fix some bugs
write the unit testing and test the main PHP classes. (NexyCrypt and PrivateKey).

Here are the results about code coverage and travis-ci.

code coverage
travis-vi

peter279k · 2016-10-04T12:56:40Z

I complete the all unit testing and solve the verify problem. Please review it and give some suggestions. Thanks.

soullivaneuh · 2016-10-04T15:38:34Z

example_http01.php

    }
 } catch (AcmeApiException $e) {
-    var_dump($e->getDetails());
+    echo $e->getDetails();


Maybe add a newline after this?

soullivaneuh · 2016-10-04T15:39:01Z

generate_fake.php

-if ($argc < 3) {
-    echo 'You have to pass domain and step arguments.'.PHP_EOL;
+if ($argc !== 2) {
+    echo 'You have to pass too many arguments.'.PHP_EOL;


What do you mean? This sentence is not understandable.

Yeah, you're right . This syntax is not readable so I use > to make syntax easy to understand.

soullivaneuh · 2016-10-04T15:40:01Z

phpunit.xml.dist

        <whitelist>
-            <file suffix=".php">./src/NexyCrypt.php</file>
-            <file suffix=".php">./src/PrivateKey.php</file>
+            <directory suffix=".php">./src/</directory>


No need for suffix key.

Please use this file: https://github.com/nexylan/slack-bundle/blob/master/phpunit.xml.dist

We have the same for all Nexylan's projects. 😉

Ok, I remove the suffix attribute.

soullivaneuh · 2016-10-04T15:42:11Z

src/NexyCrypt.php

     * @return ResponseInterface
     */
-    private function request($method, $uri, array $options = ['verify' => false])
+    private function request($method, $uri, array $options = ['verify' =>  __DIR__.'/cacert.pem'])


The problem is still the same: Passing a array as default argument.

BTW, if you pass a path, the option name should be changed to be more consistent.

I think it's a proper way to solve the SSL certificate problem or what's your suggestion ?
And the which the pem file name is more consistent ?

Please see: #4 (comment)

And I'm not talking about the filename but the option name.

verify is not very appropriate for a path.

Ok, I know it. I will remove the default verify key then it should be fine.

soullivaneuh · 2016-10-04T15:42:54Z

I did a very quick review for now. Thanks for your effort.

peter279k · 2016-10-07T11:32:45Z

Hi @soullivaneuh , I have modified the code and you can review again. I think it's almost done.

soullivaneuh

What is the goal of cacert.pem?

Sorry for the very long delay.

I'll not merge it right now but will get a closer view when it will be possible.

Thanks for that1 👍

soullivaneuh · 2017-11-08T17:56:44Z

.travis.yml

+  - travis_wait php generate_fake.php 3
+
+script:
+  - vendor/bin/phpunit


Use global PHPUnit instead.

soullivaneuh · 2017-11-08T17:57:04Z

.travis.yml

+
+before_script:
+  - composer require phpunit/phpunit
+  - travis_wait php generate_fake.php 0


What is the goal of this?

soullivaneuh · 2017-11-08T17:59:07Z

tests/ftpserver.json

@@ -0,0 +1,6 @@
+{
+    "ftpserver": "nexycrypt.96.lt",


What is that? A service to create temporary servers?

It looks like a web server just created by you. Am I right?

Yes, I just want to do the real testing so I create this FTP server.
But I'm not sure this FTP server is available now.

It is. And I'm sure you understand we can't rely on a real external server.

Plus, you should change the credentials, this is not secure at all. 😕

IMO we should just mock the http request. Also see #7

According to this post, perhaps we can let the FTP user name and password being the encrypted environment variables.

What do you think?Or just use the mock to pass this testing?

I think we should use mock for global testing. For that, you should wait #7 to be merged and use this: https://github.com/php-http/mock-client

At least, we may create a single real test with HTTP challenge using ngrok.

I'll also try to setup simple test on my side for the bridge.

soullivaneuh · 2017-11-08T17:59:17Z

tests/account.key

@@ -0,0 +1,15 @@
+-----BEGIN RSA PRIVATE KEY-----


Should be under a fixtures folder.

soullivaneuh · 2017-11-08T18:01:22Z

Also, could you please make an interactive rebase to have a clearer git history?

soullivaneuh · 2017-11-08T18:08:11Z

.travis.yml

+before_script:
+  - composer require phpunit/phpunit
+  - travis_wait php generate_fake.php 0
+  - travis_wait php generate_fake.php 1


travis_wait is not necessary here.

soullivaneuh · 2017-11-08T18:08:42Z

generate_fake.php

@@ -0,0 +1,105 @@
+<?php
+
+// generate the fake .well-known folder and upload the folder to the testing web hosting.


soullivaneuh · 2017-11-08T18:09:16Z

generate_fake.php

+$domain = $accounts['ftpserver'];
+
+// First commented line is for production.
+//$client = new NexyCrypt();


Commented code should not be here. Plus, I don't think this should be used for production.

peter279k added 30 commits September 30, 2016 11:17

add editor config and phpunit lib

1dbca17

use phpunit ^5.5

0fc70e5

fix the HTTPoxy vulnerability

8ee6040

modify the editorconfig

babd232

add unit testing (part)

03a0c15

complete the unit testing (initial)

6d27628

complete the exception unit testing

5310c40

modify the domain name and add encrypted user & password

f345bee

add travis command for testing

1e6244f

make generate_faek no time limit

55e794f

move to install section

faaf57d

move to before_script section

9b20f81

move to script section

ba81b74

fix the syntax error

af0f307

add createKey and step 0

6f4837d

add step0 (step 0 1 2 3)

0af0489

add one line "brefore install" section

ffd8821

move to before_install section

afe7c9f

move to before script section

a9a2654

modify upload file path and remote file path

fe74ef2

modify the steps

e8a8388

modify the steps add run step 2

3824bae

add getPrivateKey

1138551

add getPrivateKey before post request

bd5edbc

modify steps

a645dee

add "" avoid access denied

b9b071f

add ftp_connect in generate_fake.php

00c74b4

modify variable

7502bed

add the $domain value

edafd33

modify ftp_connect

fd481c3

peter279k added 19 commits October 3, 2016 16:37

change the editorconfig file

83c3a28

remove the phpunit from the project

a62af70

revert the autoload inline

72d5476

replace the var_dump to echo

c6308a9

rename the phpunit.xml to phpunit.xml.dist

a9b1719

modify some code style

c3096f6

remove /** @test */

2c15dc1

recover the annotation

e2a5f32

fix the phpunit command

280b289

checkout the phpunit version

60a7046

add phpunit version

e9266ec

replace annotation with prefix keyword test

7c5c0d6

remove load xdebug setting

a6ea9e5

do unit testing (ver2)

5813b3d

add cacert.pem

c5e76b9

complete the unit testing

1f85d38

add cacert.pem and fix the verify SSL cert problem

2fa9cb2

add pass arguments and execute method

99a98d5

get the correct method arg

cc3884b

read ftpserver.json to get the domain name

97ac8f0

soullivaneuh suggested changes Oct 4, 2016

View reviewed changes

peter279k added 3 commits October 5, 2016 00:00

fix some little issues

ae358c6

fix arg issue

bee3ed6

remove the default verify key

93efbe3

soullivaneuh suggested changes Nov 8, 2017

View reviewed changes

		@@ -0,0 +1,105 @@
		<?php

		// generate the fake .well-known folder and upload the folder to the testing web hosting.

Conversation

peter279k commented Oct 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change log

Uh oh!

peter279k commented Oct 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soullivaneuh commented Oct 4, 2016

Uh oh!

peter279k commented Oct 7, 2016

Uh oh!

soullivaneuh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soullivaneuh commented Nov 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

peter279k commented Oct 3, 2016 •

edited

Loading