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.

Sign up for GitHub

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

Jump to bottom

Generate a shared library linked to integration_test_target #124

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Generate a shared library linked to integration_test_target #124

wants to merge 1 commit into from

Conversation

ttreyer
Copy link
Contributor

@ttreyer ttreyer commented Apr 18, 2023 •
edited
Loading

Summary

Extend our integration test suite by linking a generated shared library to the integration_test_target. This enable us testing and validating that oid handles shared libraries. We can add definitions in the shared library using the keys definitions_shlib and raw_definitions_shlib.

Test plan

There are no tests using the new shlib definitions, yet. However, we can see this PR is not affecting our existing tests:

$ make configure-devel
[...]

$ make test
[...]
100% tests passed, 0 tests failed out of 420

Total Test time (real) =  75.04 sec

@ttreyer ttreyer requested a review from ajor April 18, 2023 15:56
@ttreyer ttreyer force-pushed the ttreyer/test-shlib branch from 390f0fc to 59db4f7 Compare April 18, 2023 16:58
ajor
ajor approved these changes Apr 24, 2023
View reviewed changes
test/integration/gen_tests.py
from textwrap import dedent

for config in test_configs:
ns = config["suite"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ns = config["suite"]
ns = get_namespace(config["suite"])

test/integration/gen_tests.py
headers.update(config.get("includes_shlib", []))
add_headers(f, custom_headers=headers, thrift_headers=[])

from textwrap import dedent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀
nice

test/integration/gen_tests.py
with open(output_target_name, "w") as f:
headers = set()
for config in test_configs:
headers.update(config.get("includes_shlib", []))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we just use the same "includes" for the shared lib? It'd simplify writing the test configs and the worst that'll happen is that we'll get unnecessary includes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, "includes_shlib" should be documented too

@ttreyer ttreyer marked this pull request as draft April 24, 2023 13:22
@ajor
Copy link
Contributor

ajor commented May 19, 2023

I had to deal with a similar issue this week for testing Split DWARF (dwp). The approach I went with was to create a target executable independently from the test framework, and then just tell the test framework to probe this custom binary.

My change to the test framework is here: #140
The other changes are not open-source, but were entirely to the build system: just build a target executable and then set TARGET_EXE_PATH to point to it when building the test framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajor ajor ajor approved these changes

@JakeHillion JakeHillion JakeHillion approved these changes

Assignees
No one assigned
Labels
cla signed testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ttreyer @ajor @JakeHillion @facebook-github-bot