Fix #2238 - Renew source files after preGenerateCommands #2239
Conversation
| import std.path : absolutePath; | ||
| import std.file : mkdirRecurse, rmdirRecurse, write; | ||
|
|
||
| mkdirRecurse("dubtest/preGen/source"); |
There was a problem hiding this comment.
I don't think the dub unittests write to the filesystem. It would probably be better to use the tests folder test system with shell scripts.
There was a problem hiding this comment.
Is that a good enough reason? I think it's a lot better to unit test even if it requires writing to the file system, especially as the only way I can get this to manifest in real life is with two dub packages where one depends on the other.
There was a problem hiding this comment.
my main reason would be that all the existing similar tests use the test folder (shell scripts or D scripts, optionally with their own folders where they can do whatever they want) and I don't think dub has such tests inside the unittests.
Additionally the test runner in the tests folder is more easily usable with isolating tests and debugging issues.
Example test script:
https://github.com/dlang/dub/blob/059f7a2a16cf7a50a5c68dc295c796482116ff87/test/environment-variables.script.d
and test folder:
https://github.com/dlang/dub/tree/059f7a2a16cf7a50a5c68dc295c796482116ff87/test/environment-variables
while this unittest is nice in testing the issue in isolation, (as unittests should do) the test folder in general is quite useful as it runs the whole dub executable with the full generator and stuff, so issues that are related to this, but not exactly this issue, may be found more easily.
There was a problem hiding this comment.
I get all that - I'm arguing that the way in this PR is better.
There was a problem hiding this comment.
Can we please merge? I don't think "We've always done it this way" is a valid argument.
|
I presume using inotify on Linux and corresponding mechanism on Windows could be used here to only renew sources files that were actually changed or created. |
That's a lot more complicated for, I think, little benefit. This works. |
|
Is this about making dub aware of modified and/or added source files? |
|
yes, if modified through preGenerateCommands, to fix #2238 |
No description provided.