Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

ResourceStateChecker. Removed addition of one extra second to getModificationTime. #1

Closed
wants to merge 1 commit into from

2 participants

Yaroslav Kiliba Konstantin Kudryashov
Yaroslav Kiliba

Bug fix: no
Feature addition: ?
Backwards compatibility break: ?
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Relates to: symfony/symfony#3961

All tests have been passed without that second, so i assume it could be deleted.

Yaroslav Kiliba

and i have a question.
https://github.com/everzet/symfony/blob/resource-watcher-rebuilt/src/Symfony/Component/ResourceWatcher/Tests/StateChecker/DirectoryStateCheckerTest.php#L81 :

<?php
    public function testDeepFileDeleted()
    {
        //...
        $foo
            ->expects($this->any())
            ->method('getFilteredResources')
            ->will($this->returnValue(array(
                $foobar = $this->createFileResourceMock(array(true, false))
            )));

Probably it should return that file only once. Second time the file would be deleted ($this->touchResource($foobar, false);) and NewDirectoryStateChecker::createNewDirectoryChildCheckers(calls at https://github.com/everzet/symfony/blob/resource-watcher-rebuilt/src/Symfony/Component/ResourceWatcher/StateChecker/NewDirectoryStateChecker.php#L66) couldn't return it in a real situation 'cause Symfony/Component/Config/Resource/DirectoryResource::getFilteredResources returns only existent files/dirs.

Yaroslav Kiliba

testDeepFileCreated has this issue too.
Once the tests are corrected I think we can simlify this check https://github.com/everzet/symfony/blob/resource-watcher-rebuilt/src/Symfony/Component/ResourceWatcher/StateChecker/NewDirectoryStateChecker.php#L70 :

if (!isset($this->childs[$resourceId]) && $resource->exists()) {

, get rid of $resource->exists()

Yaroslav Kiliba

And do we need a untrack method? In case someone wants to track the resources s/he added before except some of them.

    $resourceWatcher->untrack($trackingId);
Yaroslav Kiliba Dattaya referenced this pull request in symfony/symfony
Closed

[2.2] [WIP] ResourceWatcher component refactored #3961

Konstantin Kudryashov
Owner

1.Probably it should return that file only once. Second time the file would be deleted > Agree
2. Once the tests are corrected I think we can simlify this check > We could try
3. And do we need a untrack method? > Maybe

Yaroslav Kiliba

Looks like I found a bug - try to create a subdir in a directory, track the directory, then delete the subdir and create new file with the same name as a subdir's one.

Example:

<?php
    public function testTrackSimpleDirChanges()
    {
        $tracker = $this->getTracker();

        mkdir($directory = $this->tmpDir.'/bar');
        mkdir($sub_dir = $directory.'/sub');
        $tracker->track(new TrackedResource('bar', $resource = new DirectoryResource($directory)));

        rmdir($sub_dir);
        touch($sub_dir);
        $this->sleep();

        $events = $tracker->getEvents();
        $this->assertCount(2, $events);
        echo (string) $events[0]->getResource(); //tmp/sf2_resource_watcher_tests/bar/sub/sub

}

$events[0]->getResource() should return tmp/sf2_resource_watcher_tests/bar/sub, not tmp/sf2_resource_watcher_tests/bar/sub/sub

Yaroslav Kiliba
  1. Once the tests are corrected I think we can simlify this check > We could try

have simplified it, will probably sent a pr tomorrow with cs fixes as well

Konstantin Kudryashov everzet closed this
Konstantin Kudryashov everzet referenced this pull request from a commit
Fabien Potencier fabpot merged branch romainneutron/FilesystemExceptions (PR #4330)
Commits
-------

a20fc68 Merge pull request #1 from SamsonIT/FilesystemExceptions
8eca661 [FileSystem] explains possible failure of symlink creation in windows
b1f8744 Add Changelog BC Break note
24eb396 [Filesystem] Added few new behaviors:

Discussion
----------

[Filesystem] Consistence and enhancements for Filesystem

Bug fix: no
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: None
License of the code: MIT

This PR adds features and introduce a backward compatibility break.

features :
- whenever an action fails, a \RuntimeException is thrown
- add access to the second and third arguments of ``touch`` function
- add a recursive option for chmod
- add a chown method
- add a chgrp method

The backward compatibility break happens in the mkdir method : Before this PR, a boolean is returned ; true if all directories were created, false otherwise.
It now returns nothing.

---------------------------------------------------------------------------

by travisbot at 2012-05-18T14:26:42Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into 1e15f21).

---------------------------------------------------------------------------

by fabpot at 2012-05-20T02:40:28Z

To be consistent, we should throw exception whenever some operation fails.

---------------------------------------------------------------------------

by romainneutron at 2012-05-20T21:10:23Z

I fix the consistency ; mkdir now throws an exception if a directory creation fails.
This introduce a BC break, see PR message which has been updated with all features and BC break.

Added chgrp and chown methods
Add options for touch
Add recursive option for chmod

---------------------------------------------------------------------------

by travisbot at 2012-05-20T21:11:47Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into 1407f11).

---------------------------------------------------------------------------

by travisbot at 2012-05-22T10:49:06Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into 517ae43).

---------------------------------------------------------------------------

by travisbot at 2012-05-22T10:58:10Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into 517ae43).

---------------------------------------------------------------------------

by travisbot at 2012-05-22T11:18:44Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into 517ae43).

---------------------------------------------------------------------------

by travisbot at 2012-05-23T18:21:47Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into 517ae43).

---------------------------------------------------------------------------

by travisbot at 2012-05-23T18:26:19Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into 517ae43).

---------------------------------------------------------------------------

by travisbot at 2012-05-29T16:07:26Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e into adf07f1).

---------------------------------------------------------------------------

by travisbot at 2012-05-29T16:19:38Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa into adf07f1).

---------------------------------------------------------------------------

by romainneutron at 2012-06-01T14:30:45Z

Any news about this PR ?

---------------------------------------------------------------------------

by stloyd at 2012-06-08T09:57:39Z

@romainneutron You need to [squash](http://www.silverwareconsulting.com/index.cfm/2010/6/6/Using-Git-Rebase-to-Squash-Commits) your commits, and add more proper message in squashed commit i.e.:

> [Filesystem]  Added few new behaviors:
* whenever an action fails, a `RuntimeException` is thrown
* add access to the second and third arguments of `touch()` function
* add a recursive option for `chmod()`
* add a `chown()` method
* add a `chgrp()` method

> BC break: `mkdir()` function throw exception in case of failture instead of returning Boolean value.

---------------------------------------------------------------------------

by romainneutron at 2012-06-08T10:59:55Z

@stloyd squash done !

---------------------------------------------------------------------------

by travisbot at 2012-06-08T11:26:20Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 into f8e68e5).

---------------------------------------------------------------------------

by travisbot at 2012-06-08T11:41:45Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 into f8e68e5).

---------------------------------------------------------------------------

by romainneutron at 2012-06-09T11:42:24Z

I've added documentation to the Filesystem component : symfony/symfony-docs#1439

---------------------------------------------------------------------------

by travisbot at 2012-06-09T16:47:20Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 into f8a09db).

---------------------------------------------------------------------------

by stloyd at 2012-06-13T14:47:31Z

@romainneutron You probably need to rebase your code as some changes were merge into master for `Filesystem`.

---------------------------------------------------------------------------

by romainneutron at 2012-06-13T15:17:31Z

@stloyd rebase OK !

by the way, do you have any idea when/if this PR will be merged ?

---------------------------------------------------------------------------

by travisbot at 2012-06-13T15:20:44Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 into c07e916).

---------------------------------------------------------------------------

by fabpot at 2012-06-16T16:40:50Z

You need to add a note about the BC breaks in the CHANGELOG file.

---------------------------------------------------------------------------

by fabpot at 2012-06-16T16:43:20Z

Also, instead of using `\RuntimeException`, I would create a custom exception like we have done in other components (an interface + a RuntimeException that implements the interface and extends \RuntimeException). The exception name can be something like `IOException`.

---------------------------------------------------------------------------

by travisbot at 2012-06-18T10:11:20Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into 0b8b76b).

---------------------------------------------------------------------------

by stloyd at 2012-06-18T10:14:52Z

@fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR)

---------------------------------------------------------------------------

by romainneutron at 2012-06-18T10:29:20Z

@fabpot @stloyd the latest push was just a rebase push for PR 4577 (symfony#4577)
I'm currently fixing the Exception and changelog things, I'll push very soon

---------------------------------------------------------------------------

by romainneutron at 2012-06-18T10:44:38Z

@fabpot I've added the exception and the exception interface, add the changelog info

---------------------------------------------------------------------------

by travisbot at 2012-06-18T10:53:34Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into 0b8b76b).

---------------------------------------------------------------------------

by romainneutron at 2012-06-18T11:08:43Z

As reported by @stloyd the PR is failing due to an issue in the master, I re-push and trig the PR build when this issue is solved

---------------------------------------------------------------------------

by travisbot at 2012-06-18T11:16:58Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into 0b8b76b).
55f682c
Konstantin Kudryashov everzet referenced this pull request from a commit
Fabien Potencier fabpot merged branch bamarni/2.0 (PR #6154)
This PR was merged into the 2.0 branch.

Commits
-------

f0743b1 Merge pull request #1 from pylebecq/2.0
555e777 [FrameworkBundle] Added tests for trusted_proxies configuration.
a0e2391 [FrameworkBundle] used the new method for trusted proxies

Discussion
----------

[FrameworkBundle] used the new method for trusted proxies

This makes the framework bundle using the new method from the request class.

---------------------------------------------------------------------------

by fabpot at 2012-12-05T10:38:20Z

As this is a sensitive issue, can you add some tests? Thanks.

---------------------------------------------------------------------------

by bamarni at 2012-12-06T13:00:24Z

Well I don't know why it fails on travis, I can't run the full test suite locally because of a segfault but ```phpunit src/Symfony/Bundle/``` marks all the tests as passing.

---------------------------------------------------------------------------

by fabpot at 2012-12-06T13:08:11Z

But it looks like the failing tests come from what you've changed.

---------------------------------------------------------------------------

by bamarni at 2012-12-06T13:29:33Z

Yes, I'm not saying it's not my fault but I can't reproduce this as locally it tells me they pass, I'll try to fix this this evening.

---------------------------------------------------------------------------

by bamarni at 2012-12-06T17:49:28Z

Apparently it fails only when running the whole testsuite, looking at other travis builds I can see this one on 2.0 : https://travis-ci.org/symfony/symfony/jobs/3495511 which fails in a similar way than here (https://travis-ci.org/symfony/symfony/jobs/3530928). Because of a place trying to access an undefined $_SERVER key : ```PHP Notice:  Undefined index: SCRIPT_NAME ...``` but I can't find where, and the stack trace references some phpunit classes.

I'd be happy if someone could give me some pointers in here as I don't have any clue about how to fix this..

---------------------------------------------------------------------------

by bamarni at 2012-12-06T18:00:57Z

As a consulsion I'd say I can't run the whole testsuite locally (it fails even when I revert my commit), so there is no reliable way for me to fix this, if anyone is up for continuing this feel free.

---------------------------------------------------------------------------

by fabpot at 2012-12-11T09:47:48Z

@bamarni Can you just update this PR with the code change and no tests at all? I will then finish the PR. Thanks.

---------------------------------------------------------------------------

by bamarni at 2012-12-11T16:58:17Z

@fabpot: thanks for helping me out on this, hope you won't run into the same issue!
c0fc33d
Konstantin Kudryashov everzet referenced this pull request from a commit
Fabien Potencier fabpot merged branch jakzal/form-tests-fix (PR #6517)
This PR was merged into the 2.1 branch.

Commits
-------

81967f6 [Form] Fixed failing tests for DateTimeToStringTransformer.

Discussion
----------

[Form] Fixed failing tests for DateTimeToStringTransformer

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -

Tests were only failing at the end of the month. Since February was used in the test cases, date was being moved to the next month (February has less days than other months).

If a day is not passed, \DateTime's constructor will set it to the first day of the month:

```php
var_dump(new \DateTime('2010-02'));

object(DateTime)#1 (3) {
  ["date"]=>
  string(19) "2010-02-01 00:00:00"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(13) "Europe/London"
}
```
\DateTime is used in the test assertions.

However, DateTimeToStringTransformer::reverseTransform() uses \DateTime::createFromFormat(), which sets a missing day to the current day:

```php
var_dump(\DateTime::createFromFormat("Y-m", '2010-02'));

object(DateTime)#1 (3) {
  ["date"]=>
  string(19) "2010-03-01 20:09:26"
  ["timezone_type"]=>
  int(3)
  ["timezone"]=>
  string(13) "Europe/London"
}
```

I changed the date in the test case to avoid failures. If we need to be sure that month's not going to be changed, I'll update my PR.
f4f2ba8
Konstantin Kudryashov everzet referenced this pull request from a commit
Fabien Potencier fabpot merged branch tiagobrito/master (PR #6693)
This PR was merged into the master branch.

Commits
-------

0d6be2e Added Portuguese (Portugal) translation to Security
87df0b7 Merge pull request #1 from symfony/master

Discussion
----------

Add Portuguese from Portugal Security translation

Translation file for portuguese from Portugal added in Security component translations
8c4245f
Konstantin Kudryashov everzet referenced this pull request from a commit
Fabien Potencier fabpot merged branch kriswallsmith/class-loader/idempotent (PR #7245)
This PR was merged into the 2.1 branch.

Commits
-------

27cc0df Merge pull request #1 from merk/class-loader/idempotent
95af84c Fixed test to use Reflection
bb08247 [ClassLoader] tweaked test
73bead7 [ClassLoader] made DebugClassLoader idempotent

Discussion
----------

[ClassLoader] made DebugClassLoader idempotent

The DebugClassLoader will wrap itself if `enable()` is called multiple time, such as when running functional tests.

Please merge to 2.2 and master ASAP.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | ~
| License       | MIT
| Doc PR        | ~

---------------------------------------------------------------------------

by kriswallsmith at 2013-03-07T16:38:55Z

ping @fabpot: this will speed up lots of functional tests :)

---------------------------------------------------------------------------

by kriswallsmith at 2013-03-08T04:51:51Z

@fabpot fixed by @merk
ee495f8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2012
  1. Yaroslav Kiliba
This page is out of date. Refresh to see the latest.
2  src/Symfony/Component/ResourceWatcher/ResourceWatcher.php
View
@@ -147,7 +147,7 @@ public function isWatching()
}
/**
- * Starts wathing on tracked resources.
+ * Starts watching on tracked resources.
*
* @param integer $checkInterval check interval in microseconds
* @param integer $timeLimit maximum watching time limit in microseconds
6 src/Symfony/Component/ResourceWatcher/StateChecker/NewDirectoryStateChecker.php
View
@@ -77,7 +77,7 @@ public function getChangeset()
);
}
- // check for new direcotry changes
+ // check for new directory changes
if ($checker instanceof NewDirectoryStateChecker) {
foreach ($checker->getChangeset() as $change) {
if ($this->supportsEvent($change['event'])) {
@@ -93,7 +93,7 @@ public function getChangeset()
}
/**
- * Reads files and subdirectories on provided resource path and transform them to resources.
+ * Reads files and subdirectories on provided resource path and transforms them to resources.
*
* @param DirectoryResource $resource
*
@@ -114,7 +114,7 @@ protected function createDirectoryChildCheckers(DirectoryResource $resource)
}
/**
- * Reads files and subdirectories on provided resource path and transform them to resources.
+ * Reads files and subdirectories on provided resource path and transforms them to resources.
*
* @param DirectoryResource $resource
*
6 src/Symfony/Component/ResourceWatcher/StateChecker/ResourceStateChecker.php
View
@@ -35,7 +35,7 @@
public function __construct(ResourceInterface $resource, $eventsMask = FilesystemEvent::IN_ALL)
{
$this->resource = $resource;
- $this->timestamp = $resource->getModificationTime() + 1;
+ $this->timestamp = $resource->getModificationTime();
$this->eventsMask = $eventsMask;
$this->deleted = !$resource->exists();
}
@@ -71,7 +71,7 @@ public function getChangeset()
if ($this->deleted) {
if ($this->resource->exists()) {
- $this->timestamp = $this->resource->getModificationTime() + 1;
+ $this->timestamp = $this->resource->getModificationTime();
$this->deleted = false;
if ($this->supportsEvent($event = FilesystemEvent::IN_CREATE)) {
@@ -91,7 +91,7 @@ public function getChangeset()
);
}
} elseif (!$this->resource->isFresh($this->timestamp)) {
- $this->timestamp = $this->resource->getModificationTime() + 1;
+ $this->timestamp = $this->resource->getModificationTime();
if ($this->supportsEvent($event = FilesystemEvent::IN_MODIFY)) {
$changeset[] = array(
6 src/Symfony/Component/ResourceWatcher/Tests/StateChecker/FileStateCheckerTest.php
View
@@ -32,7 +32,7 @@ public function testNoChanges()
$resource
->expects($this->once())
->method('isFresh')
- ->with(12)
+ ->with(11)
->will($this->returnValue(true));
$this->assertEquals(array(), $checker->getChangeset());
@@ -62,7 +62,7 @@ public function testModified()
$resource
->expects($this->once())
->method('isFresh')
- ->with(12)
+ ->with(11)
->will($this->returnValue(false));
$this->assertEquals(
@@ -83,7 +83,7 @@ public function testConsecutiveChecks()
$resource
->expects($this->once())
->method('isFresh')
- ->with(12)
+ ->with(11)
->will($this->returnValue(false));
$this->assertEquals(
Something went wrong with that request. Please try again.