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

ci: run c++ tests in an integration build #1637

Closed
3 tasks done
ianna opened this issue Aug 26, 2022 · 3 comments · Fixed by #2981
Closed
3 tasks done

ci: run c++ tests in an integration build #1637

ianna opened this issue Aug 26, 2022 · 3 comments · Fixed by #2981
Assignees
Labels
feature New feature or request

Comments

@ianna
Copy link
Collaborator

ianna commented Aug 26, 2022

Description of new feature

There are four c++ tests in tests-cpp that are built, but not run. They require C++17.

Tasks

@ianna ianna added the feature New feature or request label Aug 26, 2022
@jpivarski
Copy link
Member

Is this still true? It's an old issue, and the testing situation may have changed.

If the issue is no longer relevant, please close the issue. Thanks!

@ianna
Copy link
Collaborator Author

ianna commented Jan 22, 2024

Is this still true? It's an old issue, and the testing situation may have changed.

If the issue is no longer relevant, please close the issue. Thanks!

I think, the issue is still relevant. Only 3 out of four tests are run:

Run import os
  import os
  import pathlib
  import subprocess
  for path in pathlib.Path("build/tests/bin").glob("test_*"):
      if path.is_file():
          print(f"Running {path.name}", flush=True)
          print("::group::Test output", flush=True)
          subprocess.run([path], check=True)
          print("::endgroup::", flush=True)
  shell: /usr/bin/python3 {0}
Running test_1560-builder-options
Test output
Running test_1542-growable-buffer
Test output
Running test_1494-layout-builder
Test output

I understand, that the array builder test is not a header-only test - it needs to be linked to libawkward.so. Also, we need to check that the dependencies are propagated. For example, changes to the header-only should trigger rebuilding and testing of the array builder:

#include "awkward/BuilderOptions.h"

Tested in #2976, #2977, and #2978

@jpivarski
Copy link
Member

I don't think we need a C++ test for ArrayBuilder; it's tested well enough in both Python and Numba. We could just delete that one: header-only/tests/test_1542-array-builder.cpp. I just took a look at it, and it doesn't do very much, anyway.

Then we're testing everything (3 out of a total of 3), right? If so, this issue could be considered resolved once test_1542-array-builder.cpp is deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants