Skip to content

[tests] Improve dylink_testf. NFC #14992

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

Merged
merged 1 commit into from
Sep 2, 2021
Merged

[tests] Improve dylink_testf. NFC #14992

merged 1 commit into from
Sep 2, 2021

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Sep 2, 2021

Using the default for side module and output filename
mean call sites don't need so much boilerplate.

@sbc100 sbc100 requested a review from kripken September 2, 2021 14:24
@sbc100 sbc100 force-pushed the dylink_testf branch 2 times, most recently from b056f92 to 089b3df Compare September 2, 2021 14:25
Using the default for side module and output filename
mean call sites don't need so much boilerplate.
@sbc100 sbc100 requested a review from aheejin September 2, 2021 14:26
@sbc100 sbc100 enabled auto-merge (squash) September 2, 2021 14:26
@@ -4579,9 +4581,7 @@ def test_dylink_argv_argc(self):
def test_dylink_weak(self):
# Verify that weakly defined symbols can be defined in both side module and main
# module but that only one gets used at runtime.
main = test_file('core/test_dylink_weak_main.c')
side = test_file('core/test_dylink_weak_side.c')
self.dylink_testf(main, side, force_c=True, need_reverse=False)
Copy link
Member

Choose a reason for hiding this comment

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

force_c=True was deleted in the RHS.. Is this intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, force_c is only needed when the filename is not specified. In this case it doesn't do anything.

@sbc100 sbc100 merged commit ad6e356 into main Sep 2, 2021
@sbc100 sbc100 deleted the dylink_testf branch September 2, 2021 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants