Skip to content

Adding more comment for Reflection tests that does not support ASAN #75519

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 31 commits into from
Jul 29, 2024

Conversation

lamtrinhdev
Copy link
Contributor

Summary:

As reference the PR #75509 (Reflection tests are incompatible with ASAN), Adding more comment on some files in Reflection tests

Modifications:

  • validation-test/Reflection/existentials.swift
  • validation-test/Reflection/existentials_objc.swift
  • validation-test/Reflection/functions.swift
  • validation-test/Reflection/functions_objc.swift
  • validation-test/Reflection/inherits_NSObject.swift
  • validation-test/Reflection/inherits_ObjCClasses.swift
  • validation-test/Reflection/inherits_Swift.swift

Result:

Adding more "UNSUPPORTED: asan"

Adding more comment for "UNSUPPORTED: asan".
Adding more comment for "UNSUPPORTED: asan".
Adding more comment for "UNSUPPORTED: asan".
Adding more comment for "UNSUPPORTED: asan".
Adding more comment for "UNSUPPORTED: asan".
Adding more comment for "UNSUPPORTED: asan".
Adding more comment for "UNSUPPORTED: asan".
@lamtrinhdev lamtrinhdev requested a review from slavapestov as a code owner July 27, 2024 11:11
@lamtrinhdev
Copy link
Contributor Author

Dear @tbkka ,

As the PR #75509 . I would like to add some more comments on other files in "Reflection tests".

Please help me to review it.

Thanks,
Lam

@tbkka
Copy link
Contributor

tbkka commented Jul 29, 2024

Looks good. The specific problem is that %target-swift-reflection-test is now incompatible with ASAN. In looking at that, I noticed that some of the tests that use %target-swift-reflection-test do not have REQUIRES: reflection-test-support Could you please add that in as well while you're cleaning up things here?

Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
Adding comment for "// REQUIRES: reflection_test_support"
@lamtrinhdev
Copy link
Contributor Author

Dear @tbkka ,

Sure. Thank you for your advice. I updated all of them. Please help me to review it.

Thanks,
Lam

@tbkka
Copy link
Contributor

tbkka commented Jul 29, 2024

Looks great! Let's make sure it passes CI, then I'll merge it.

@tbkka
Copy link
Contributor

tbkka commented Jul 29, 2024

@swift-ci Please test

@tbkka tbkka merged commit 262719c into swiftlang:main Jul 29, 2024
5 checks passed
@tbkka
Copy link
Contributor

tbkka commented Jul 29, 2024

Thank you!

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