Skip to content

Conversation

@martin4861
Copy link
Contributor

@martin4861 martin4861 commented Oct 23, 2025

Extending the existing test suite helps for the upcoming task of refactoring the compile option handling. Also adding hedron compile commands.

@martin4861 martin4861 changed the title Add hedron and extend integration test project Add hedron and extend test suite Oct 23, 2025
@martin4861 martin4861 force-pushed the martin4861/add-hedron branch from 071a1bd to bb6cb49 Compare October 23, 2025 16:31
@martin4861 martin4861 force-pushed the martin4861/add-hedron branch from bb6cb49 to e4d3161 Compare October 23, 2025 16:32
@martin4861 martin4861 marked this pull request as ready for review October 23, 2025 16:35
@martin4861 martin4861 requested a review from a team as a code owner October 23, 2025 16:35
@martin4861 martin4861 requested a review from sbmueller October 23, 2025 16:57
Copy link
Contributor

@sbmueller sbmueller left a comment

Choose a reason for hiding this comment

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

LGTM. Do you also want to extend the GitHub workflow to include the new functionality?


TEST(BaseMathTest, SignPlus) {
EXPECT_EQ(Math::sign(10), 1);
//EXPECT_EQ(Math::sign(-10), -1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I do not want to reach 100% coverage intentionally.

load("@rules_swiftnav//cc:defs.bzl", "UNIT", "swift_cc_test")

swift_cc_library(
cc_library(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this not use the swift macro?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to test the behavior of the toolchains, when the normal rules are used (mimic behavior from 3rd party libs)

@martin4861 martin4861 merged commit 0237e4e into main Oct 24, 2025
2 checks passed
@martin4861 martin4861 deleted the martin4861/add-hedron branch October 24, 2025 13:28
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