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

MacOS make generate fix - regenerate certain mocks using source mode instead of reflect mode #3886

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

kimorris27
Copy link
Contributor

Which issue this PR addresses:

No Jira

What this PR does / why we need it:

I found after updating my OS to Sonoma 14.7, which included updating the Command Line Tools for Xcode from 15.3 to 16.0, that make generate was failing for certain packages and was also outputting some linker warnings. I can't explain exactly why it works, but I found that having mockgen use source mode instead of reflect mode for the failing packages fixed things. The linker warnings about a deprecated -ld_classic, which were noted in the CLT for Xcode 16.0 release notes, also went away, which is interesting to note.

I could try to dig into the reasoning for this change further, but I don't think it'd be a great use of time.

I've encountered multiple issues related to reflect mode in my work recently, which makes me wonder if we should switch to using source mode throughout the repository.

Test plan for issue:

Tested locally to confirm that it fixes make generate on my Mac and unit test still work, and then ran make ci-rp to make sure it didn't break anything in the containerized process either.

Is there any documentation that needs to be updated for this PR?

N/A

How do you know this will function as expected in production?

Not a production change; only affects generation of mock interfaces that are used in unit tests.

@kimorris27
Copy link
Contributor Author

/azp run ci

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kimorris27
Copy link
Contributor Author

Skipping E2E since this PR only impacts unit tests.

Copy link
Collaborator

@tsatam tsatam left a comment

Choose a reason for hiding this comment

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

LGTM, I tested make generate on Linux and it works fine and doesn't create a diff, our CI checks back that up too.

I'd be incredibly disappointed if somehow changing our mock generation impacts production flows in any way, but out of an abundance of caution I say we should wait for green E2E on this before merge. I changed my mind, ship it without the E2E check

@kimorris27 kimorris27 merged commit 1a51bf4 into master Oct 4, 2024
18 checks passed
@kimorris27 kimorris27 deleted the kimorris27/hotfix-mockgen-issues-on-macos branch October 4, 2024 17:29
slawande2 pushed a commit that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw ready-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants