Move JIT tests to gtest #12030
Move JIT tests to gtest #12030
Conversation
goldsborough
requested review from
apaszke,
colesbury,
ebetica,
ezyang,
gchanan,
soumith and
zdevito
as code owners
test/cpp/jit/gtest.cpp
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
I... don't really understand how this structure works. So if I want to compile PyTorch without linking gtest (which would be weird), what happens to these files? |
|
@ezyang The tests from Gtest doesn't get linked into Pytorch (not weird), it gets linked into the test binaries |
goldsborough
force-pushed
the
jit-tests
branch
5 times, most recently
from
f856eb0 to
2e3ed19
Compare
goldsborough
force-pushed
the
jit-tests
branch
3 times, most recently
from
f23d301 to
ee230cf
Compare
ezyang
left a comment
ezyang
left a comment
There was a problem hiding this comment.
Sure, why not.
It's pretty weird we need the Python-enabled version, but c'est la vie, I suppose.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@peterjc123 @yf225 do you know what I have to do here for Windows? I added the |
goldsborough
force-pushed
the
jit-tests
branch
from
62e8edc to
e394ae0
Compare
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
goldsborough
force-pushed
the
jit-tests
branch
from
e394ae0 to
dd7f2a3
Compare
goldsborough
force-pushed
the
jit-tests
branch
from
dd7f2a3 to
f525931
Compare
Summary: In our #better-engineering quest of removing all uses of catch in favor of gtest, this PR ports JIT tests to gtest. After pytorch#11846 lands, we will be able to delete catch. I don't claim to use/write these tests much (though I wrote the custom operator tests) so please do scrutinize whether you will want to write tests in the way I propose. Basically: 1. One function declaration per "test case" in test/cpp/jit/test.h 2. One definition in test/cpp/jit/test.cpp 3. If you want to be able to run it in Python, add it to `runJitTests()` which is called from Python tests 4. If you want to be able to run it in C++, add a `JIT_TEST` line in test/cpp/jit/gtest.cpp Notice also I was able to share support code between C++ frontend and JIT tests, which is healthy. ezyang apaszke zdevito Pull Request resolved: pytorch#12030 Differential Revision: D10207745 fbshipit-source-id: c1e7d55d05840943bb2f92aa207082e836009416
goldsborough
force-pushed
the
jit-tests
branch
from
f525931 to
8a5e161
Compare
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
goldsborough is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: Removes test_jit.cpp, which was supposed to have been deleted in #12030 I had to move zou3519's dynamic DAG tests into `test/cpp/jit/tests.h` too. No other changes to `test_jit.cpp` seem to have happened in the meantime. zdevito Pull Request resolved: #12988 Differential Revision: D10854320 Pulled By: goldsborough fbshipit-source-id: 7ab533e6e494e34a16ce39bbe62b1150e48fcb58
3 participants
In our #better-engineering quest of removing all uses of catch in favor of gtest, this PR ports JIT tests to gtest. After #11846 lands, we will be able to delete catch.
I don't claim to use/write these tests much (though I wrote the custom operator tests) so please do scrutinize whether you will want to write tests in the way I propose. Basically:
runJitTests()which is called from Python testsJIT_TESTline in test/cpp/jit/gtest.cppNotice also I was able to share support code between C++ frontend and JIT tests, which is healthy.
@ezyang @apaszke @zdevito