Retrieve test data directory at run time

Add path service to retrieve test data directory at run time so tests can be run from any directory.

Previously the tests which read test files assume the current directory is under pdfium. Running from any other directory will break the build.

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/c0e93a9a942fe7d99800502a61d2fbb58cf9276f

[Wei Li, 2015-11-07 01:33:13]

Description was changed from

==========
Add path service to retrieve test data directory at runtime so the tests can be
run
from any directory.

Previously the tests which read files assume the current directory is
under pdfium. Running from any other directory will break the build.
==========

to

==========
Add path service to retrieve test data directory at run time so tests can be run
from any directory.

Previously the tests which read test files assume the current directory is under
pdfium. Running from any other directory will break the build.
==========

[Wei Li, 2015-11-07 01:33:14]

weili@chromium.org changed reviewers:
+ thestig@chromium.org

[Wei Li, 2015-11-07 01:38:59]

PTAL, thanks.

BTW, I feel path service can be put under other dir so that not only tests can
use it. But I have not found other use cases and reluctant to change dir
structure amid refactoring.

[Lei Zhang, 2015-11-07 01:48:51]

Mostly good. A few questions:

https://codereview.chromium.org/1408003014/diff/20001/testing/embedder_test.cpp
File testing/embedder_test.cpp (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/embedder_test.c...
testing/embedder_test.cpp:96: bool EmbedderTest::OpenDocument(const std::string&
filename,
Can you update the comment in testing/embedder_test.h ?

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.cpp (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.cpp:20: if (path.size() > 1 && path[path.size() - 1]
== PATH_SEPARATOR)
if (foo)
  return true;
return false;

--> return foo;

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.cpp:74: // Assume executables always run from
out/<Debug|Release>/, the source
BTW, did some of the code in this file come from Chromium's PathService? Do they
make the same assumption there?

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.h (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.h:19: static bool EndsWithSeparator(const
std::string& path);
Comments please.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.h:20: static bool GetExecutableDir(std::string*
path);
Can this return a string, with an empty string as the sentinel value for
failure?

Ditto below.

[Wei Li, 2015-11-07 02:35:39]

Patchset #3 (id:40001) has been deleted

[Wei Li, 2015-11-07 02:36:13]

https://codereview.chromium.org/1408003014/diff/20001/testing/embedder_test.cpp
File testing/embedder_test.cpp (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/embedder_test.c...
testing/embedder_test.cpp:96: bool EmbedderTest::OpenDocument(const std::string&
filename,
On 2015/11/07 01:48:51, Lei Zhang wrote:
> Can you update the comment in testing/embedder_test.h ?

Done.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.cpp (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.cpp:20: if (path.size() > 1 && path[path.size() - 1]
== PATH_SEPARATOR)
On 2015/11/07 01:48:51, Lei Zhang wrote:
> if (foo)
>   return true;
> return false;
> 
> --> return foo;

Done.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.cpp:74: // Assume executables always run from
out/<Debug|Release>/, the source
On 2015/11/07 01:48:51, Lei Zhang wrote:
> BTW, did some of the code in this file come from Chromium's PathService? Do
they
> make the same assumption there?

Chromium's path service is much more complex. We may not need/be able to handle
all those cases. But, yes, the assumption is same. Otherwise, how could we find
test data dir if there is no relation.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.h (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.h:19: static bool EndsWithSeparator(const
std::string& path);
On 2015/11/07 01:48:51, Lei Zhang wrote:
> Comments please.

Done.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.h:20: static bool GetExecutableDir(std::string*
path);
On 2015/11/07 01:48:51, Lei Zhang wrote:
> Can this return a string, with an empty string as the sentinel value for
> failure?
> 
> Ditto below.

Yes, it can. But we would need to write special comments while the current one
may be more intuitive about the purposes of input/return. WDYT?

[Lei Zhang, 2015-11-07 02:43:31]

lgtm

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.cpp (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.cpp:74: // Assume executables always run from
out/<Debug|Release>/, the source
On 2015/11/07 02:36:12, Wei wrote:
> On 2015/11/07 01:48:51, Lei Zhang wrote:
> > BTW, did some of the code in this file come from Chromium's PathService? Do
> they
> > make the same assumption there?
> 
> Chromium's path service is much more complex. We may not need/be able to
handle
> all those cases. But, yes, the assumption is same. Otherwise, how could we
find
> test data dir if there is no relation. 

There could be other assumptions, like looking for the .gn or AUTHORS file.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.h (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.h:20: static bool GetExecutableDir(std::string*
path);
On 2015/11/07 02:36:12, Wei wrote:
> On 2015/11/07 01:48:51, Lei Zhang wrote:
> > Can this return a string, with an empty string as the sentinel value for
> > failure?
> > 
> > Ditto below.
> 
> Yes, it can. But we would need to write special comments while the current one
> may be more intuitive about the purposes of input/return. WDYT? 

We use it in a few places. In some ways it's easier because there's no
parameters to deal with. I don't have strong feelings.

https://codereview.chromium.org/1408003014/diff/60001/testing/utils/path_serv...
File testing/utils/path_service.cpp (right):

https://codereview.chromium.org/1408003014/diff/60001/testing/utils/path_serv...
testing/utils/path_service.cpp:72: // Assume executables always run from
out/<Debug|Release>/, the source
This can probably go in the header.

https://codereview.chromium.org/1408003014/diff/60001/testing/utils/path_serv...
testing/utils/path_service.cpp:87: path->append("testing");
Ditto, write the assumption in the header.

[Wei Li, 2015-11-09 18:08:19]

thanks, will submit.

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
File testing/utils/path_service.h (right):

https://codereview.chromium.org/1408003014/diff/20001/testing/utils/path_serv...
testing/utils/path_service.h:20: static bool GetExecutableDir(std::string*
path);
On 2015/11/07 02:43:30, Lei Zhang wrote:
> On 2015/11/07 02:36:12, Wei wrote:
> > On 2015/11/07 01:48:51, Lei Zhang wrote:
> > > Can this return a string, with an empty string as the sentinel value for
> > > failure?
> > > 
> > > Ditto below.
> > 
> > Yes, it can. But we would need to write special comments while the current
one
> > may be more intuitive about the purposes of input/return. WDYT? 
> 
> We use it in a few places. In some ways it's easier because there's no
> parameters to deal with. I don't have strong feelings.

Acknowledged.

https://codereview.chromium.org/1408003014/diff/60001/testing/utils/path_serv...
File testing/utils/path_service.cpp (right):

https://codereview.chromium.org/1408003014/diff/60001/testing/utils/path_serv...
testing/utils/path_service.cpp:72: // Assume executables always run from
out/<Debug|Release>/, the source
On 2015/11/07 02:43:30, Lei Zhang wrote:
> This can probably go in the header.

Done.

https://codereview.chromium.org/1408003014/diff/60001/testing/utils/path_serv...
testing/utils/path_service.cpp:87: path->append("testing");
On 2015/11/07 02:43:30, Lei Zhang wrote:
> Ditto, write the assumption in the header.

Done.

[Wei Li, 2015-11-09 18:22:36]

Committed patchset #4 (id:80001) manually as
c0e93a9a942fe7d99800502a61d2fbb58cf9276f (presubmit successful).