Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run tests in CI #38

Open
3 tasks
kurapov-peter opened this issue May 6, 2024 · 9 comments
Open
3 tasks

Run tests in CI #38

kurapov-peter opened this issue May 6, 2024 · 9 comments
Assignees
Labels
ci Continuous integration and automation

Comments

@kurapov-peter
Copy link
Contributor

kurapov-peter commented May 6, 2024

  • gtest for integration tests - these are built by enabling -DGC_TEST_ENABLE=on. Currently, there's only one built now, run as cd test/dnnl/ && ./test_dnnl_c_interface. I think it's missing a enable_testing now for ctest. Feel free to add that or a separate target.
  • lits for our dialects - @Devjiu TBD.
  • llvm tests on llvm build - these can be run with a single command after the llvm build/install (e.g., ninja check-mlir)
@Devjiu
Copy link
Contributor

Devjiu commented May 6, 2024

To run lit tests just build "gc-check"

cmake --build . --target gc-check

Currently it's just a stub - we don't have passes and correct use-cases to run lits.

@Menooker
Copy link
Contributor

Menooker commented May 8, 2024

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

@leshikus
Copy link
Contributor

leshikus commented May 8, 2024

@Devjiu gc-check does not work for me, it reports

/bin/sh: 1: /llvm-lit: not found
FAILED: test/CMakeFiles/gc-check /runner/_work/graph-compiler/graph-compiler/build/test/CMakeFiles/gc-check 
cd /runner/_work/graph-compiler/graph-compiler/build/test && /llvm-lit -sv /runner/_work/graph-compiler/graph-compiler/build/test
ninja: build stopped: subcommand failed.

probably llvm should be rebuilt to add lit, or it should be made available in some other way. What is recommended way to resolve this?

@Devjiu
Copy link
Contributor

Devjiu commented May 8, 2024

@Devjiu gc-check does not work for me, it reports

/bin/sh: 1: /llvm-lit: not found
FAILED: test/CMakeFiles/gc-check /runner/_work/graph-compiler/graph-compiler/build/test/CMakeFiles/gc-check 
cd /runner/_work/graph-compiler/graph-compiler/build/test && /llvm-lit -sv /runner/_work/graph-compiler/graph-compiler/build/test
ninja: build stopped: subcommand failed.

probably llvm should be rebuilt to add lit, or it should be made available in some other way. What is recommended way to resolve this?

This pr added llvm-lit into build: https://github.com/intel/graph-compiler/pull/39/files
So it should be in a tarball of llvm.

another way is to install lit in pip, but I recommend use the same lit as it was in llvm.

@kurapov-peter
Copy link
Contributor Author

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

What benefit does it bring?

@Menooker
Copy link
Contributor

Menooker commented May 9, 2024

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

What benefit does it bring?

It provides a unified entry of all tests, as done in LLVM project. Most (all?) FileCheck/Python/Gtest are driven by lit.

Another benefit is that, we can reuse the gtest headers and static libraries exported by LLVM, and we don't need to manage gtest as a 3rdparty on our own. We can use the cmake function add_unittest provided by LLVM for linking gtest.

@Devjiu
Copy link
Contributor

Devjiu commented May 10, 2024

Shall we let llvm-lit to drive all of the tests including gtest? I have enabled a MLIR-related unittest at #44

What benefit does it bring?

It provides a unified entry of all tests, as done in LLVM project. Most (all?) FileCheck/Python/Gtest are driven by lit.

Another benefit is that, we can reuse the gtest headers and static libraries exported by LLVM, and we don't need to manage gtest as a 3rdparty on our own. We can use the cmake function add_unittest provided by LLVM for linking gtest.

Currently we also need gtests that test the oneDNN API integration.

If in this case everything works fine, we can consider it; if not, then the idea of a common runner for all tests does not work and a different structure needs to be discussed.

We will also need integration tests that run the pipeline starting from the pytorch model, which can also be inconvenient to embed in cmake. It seems to me that we will have several test runners in any case, since there is a need for different types of testing.

In general, such functionality at the current stage is simply a matter of taste.

@Menooker
Copy link
Contributor

In general, such functionality at the current stage is simply a matter of taste.

OK, sure! But I would suggest to drive all unittests directly testing MLIR with lit. I mean the C++ tests for the MLIR components. As it matches the MLIR upstream's way and makes it easier to upstream.

@kwasd kwasd added the ci Continuous integration and automation label Jun 5, 2024
@leshikus
Copy link
Contributor

@kurapov-peter should I add more tests to CI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration and automation
Projects
None yet
Development

No branches or pull requests

5 participants