commons-vfs
commons-vfs copied to clipboard
Call `refresh` by default in `AbstractFileObject.onChange`
I think this is a good default. Most providers won't do anything different than this. I guess? @garydgregory thoughts?
Hi @boris-petrov Run a local build before you create a PR, as this would have show you this change breaks the build. I am not sure of the semantics, needs studying... You PR would need a new test anyway to validate the change.
@garydgregory - I spent a lot of time trying to figure what's going on. I give up. This test failure is definitely a bug in the RamFileProvider
but it is difficult to fix. I changed a bunch of things and always something fails.
If you have the time, I would suggest you look into this and try fixing it.
Things that I saw which are not OK:
-
AbstractFileObject::injectType
is only overridden inRamFileObject
which seems fishy. I would try to remove this method. -
AbstractFileObject::endOutput
is a strange method. It is overridden only inRamFileObject
which, again, is not nice.
Check also #151. I fixed some more "bad" things in the code around this.
Hi @boris-petrov Please rebase on git master. TY!
@garydgregory - done!
@boris-petrov
Please rebase on master to pickup better unit test assertion messages in ProviderWriteTests
.
@garydgregory - done!
Hi @boris-petrov As you may have seen in the check list, this PR breaks the build:
Tests run: 93, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 5.007 sec <<< FAILURE! - in org.apache.commons.vfs2.provider.ram.test.RamProviderTestCase
testListener(org.apache.commons.vfs2.test.ProviderWriteTests) Time elapsed: 0.007 sec <<< FAILURE!
junit.framework.AssertionFailedError: ram:///write-tests/newfile
Please rebase on master to use the latest but I am thinking that it might be your actual changes that brake the test.
Also this PR does that have a test or tests that fail without your change to the main side of the source tree.
@garydgregory - I rebased on master.
Please see this comment. As I said there, the failing test is a real issue in the RAM
provider.
Also, please merge #151 before this. It simplifies some code and will help with debugging this issue here.
As for tests, this is not fixing any issues, it's just a convenience for some file systems that won't have to do anything "manually" onChange
.
Hi @boris-petrov May you rebase on master?
@garydgregory done!
@boris-petrov #151 is in, but the builds still fail here. Any ideas?
Well, as I've said in a previous comment, there is some bug in the RamFileProvider
. I've forgotten what the problem is. I guess someone would need to debug it.
OK, then if someone wants to get this build green, they know where to dig.