Skip to content
This repository was archived by the owner on Jul 28, 2024. It is now read-only.

Make sure the test-logs directory exists#1379

Merged
borekb merged 1 commit intoversionpress:masterfrom
candrews:test-logs-exists
Mar 4, 2019
Merged

Make sure the test-logs directory exists#1379
borekb merged 1 commit intoversionpress:masterfrom
candrews:test-logs-exists

Conversation

@candrews
Copy link
Copy Markdown
Contributor

@candrews candrews commented Mar 4, 2019

The directory must exist. If it doesn't, tests fail to run with this output:

$ npm run tests:full

> versionpress-dev-workspace@1.0.0 tests:full /home/candrews/projects/versionpress
> docker-compose run --rm tests-with-wordpress ../vendor/bin/phpunit -c phpunit.xml

Starting versionpress_mysql-for-tests_1 ... done
Starting versionpress_selenium-hub_1    ... done
Starting versionpress_wordpress-for-tests_1 ... done
RuntimeException: Logging directory not found or is not absolute path. in /opt/versionpress/vendor/tracy/tracy/src/Tracy/Debugger.php:162
Stack trace:
#0 /opt/versionpress/tests/phpunit-bootstrap.php(13): Tracy\Debugger::enable(false, '/var/opt/versio...')
#1 /opt/versionpress/vendor/phpunit/phpunit/src/Util/Fileloader.php(52): include_once('/opt/versionpre...')
#2 /opt/versionpress/vendor/phpunit/phpunit/src/Util/Fileloader.php(36): PHPUnit_Util_Fileloader::load('/opt/versionpre...')
#3 /opt/versionpress/vendor/phpunit/phpunit/src/TextUI/Command.php(914): PHPUnit_Util_Fileloader::checkAndLoad('/opt/versionpre...')
#4 /opt/versionpress/vendor/phpunit/phpunit/src/TextUI/Command.php(739): PHPUnit_TextUI_Command->handleBootstrap('/opt/versionpre...')
#5 /opt/versionpress/vendor/phpunit/phpunit/src/TextUI/Command.php(127): PHPUnit_TextUI_Command->handleArguments(Array)
#6 /opt/versionpress/vendor/phpunit/phpunit/src/TextUI/Command.php(116): PHPUnit_TextUI_Command->run(Array, true)
#7 /opt/versionpress/vendor/phpunit/phpunit/phpunit(52): PHPUnit_TextUI_Command::main()
#8 {main}
Unable to log error: Directory is not specified.
npm ERR! code ELIFECYCLE
npm ERR! errno 255
npm ERR! versionpress-dev-workspace@1.0.0 tests:full: `docker-compose run --rm tests-with-wordpress ../vendor/bin/phpunit -c phpunit.xml`
npm ERR! Exit status 255
npm ERR! 
npm ERR! Failed at the versionpress-dev-workspace@1.0.0 tests:full script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/candrews/.npm/_logs/2019-03-04T16_31_07_728Z-debug.log

@candrews candrews mentioned this pull request Mar 4, 2019
Merged
@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 4, 2019

On my machine, I can do rm -rf dev-env/test-logs && npm run tests:full without any issues but it's possible that this is something OS-dependent.

Merging, thank you.

@borekb borekb merged commit 3d0b91d into versionpress:master Mar 4, 2019
@borekb borekb added this to the 4.0 milestone Mar 4, 2019
@borekb borekb added the scope: tests Testing code. For infrastructure (CI, etc.), use "dev-infrastructure". label Mar 4, 2019
@borekb borekb mentioned this pull request Mar 5, 2019
Closed
@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 5, 2019

I'm revisiting this after returning from holiday and noticed two things:

  • It's a bit strange that the dev-env/test-logs folder is now part of the repo while other similar folders like dev-env/wp or dev-env/wp-for-tests aren't. This inconsistency should be resolved.
  • The path hasn't been removed from dev-env/.gitignore.

I'm also questioning the original motivation for this PR, maybe we're doing something wrong when it comes to permissions in our dockerized dev stack, I don't know. See Testing is problematic on Linux #1383.

@candrews
Copy link
Copy Markdown
Contributor Author

candrews commented Mar 5, 2019 

* The path hasn't been removed from [`dev-env/.gitignore`](https://github.com/versionpress/versionpress/blob/b14fe014360ae36597ac2fd18effa801de566f31/dev-env/.gitignore).

That's intentional. The goal is just create the directory - we still don't want changes to it in source control.

@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 5, 2019

Oh sure, sorry.

@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 9, 2019

This is causing trouble for me as the .keep files might be deleted during test runs and my local Git will then report dev-env/test-logs/.keep and dev-env/test-logs/.tracy/.keep as deleted, which is incorrect.

I think we should revert this and rather focus on why Docker on Linux won't create the necessary folders as on other systems. This might be related to #1383.

@borekb borekb mentioned this pull request Mar 10, 2019
Merged
@borekb
Copy link
Copy Markdown
Member

borekb commented Mar 10, 2019

Update: #1396 (commit fe8ea7f) removes Tracy so .tracy/.keep is no longer necessary.

As for test-logs itself, I now believe that the issue was caused by incorrect username in Dockerfile which is being addressed in #1397.

This PR is effectively reverted by the aforementioned commit.

@borekb borekb mentioned this pull request Mar 10, 2019
Merged
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

scope: tests Testing code. For infrastructure (CI, etc.), use "dev-infrastructure".

Projects

None yet

Milestone

4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@candrews @borekb