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

Have computed var properties also use the queue to add invocations safely #334

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

welshm
Copy link

@welshm welshm commented Dec 20, 2022

I believe this fixes #333

@welshm
Copy link
Author

welshm commented Dec 20, 2022

@welshm
Copy link
Author

welshm commented Dec 20, 2022

@navartis @spaluchiewicz looks like you're the most recent committers? Are you reviewing pull requests for this repo?

Copy link

@maxim-chipeev maxim-chipeev left a comment

Choose a reason for hiding this comment

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

Awesome, we need this for testing in an async environment

@welshm
Copy link
Author

welshm commented Jan 5, 2023

Anything I can do to help get this reviewed and merged?

@maxim-chipeev
Copy link

@g-amichnia Hi, I saw you were the second most recent reviewer…would you be so kind as to take a look?

@cassianomonteiro
Copy link

+1 on this.

@cassianomonteiro
Copy link

@spaluchiewicz Would you be able to take a look at this?

@roelspruit
Copy link

Hi. we ran into this issue in our project. Is somebody still going to review this MR? We'd love to have a fix for this issue in an official release.

@cassianomonteiro
Copy link

@spaluchiewicz Bumping this up, would you be able to take a look? This is becoming more critical as we adopt wider usages of async/await.

@Izepe
Copy link

Izepe commented Oct 7, 2024

@spaluchiewicz Can you guys take a look a this? We are having this same issue in our project

@etiennemartin-faire
Copy link

@welshm Nice to see you working on this! I noticed that this change also requires two more changes. Some of the templates are hardcoded in swift. You can see that we'll need a similar change here as well:

return "\n\t\tget {\t\(staticModifier)invocations.append(.\(propertyCaseGetName)); return \(staticModifier)\(privatePrototypeName) ?? \(returnValue) }"

return "\n\t\tset {\t\(staticModifier)invocations.append(.\(propertyCaseSetName)(.value(newValue))); \(variable.isStatic ? "\(scope)." : "")\(privatePrototypeName) = newValue }"

I was going to re-create this PR from a Fork I created. If you could add it let me know. Our of curiosity as well. Did you manage to get the tests running locally? The project file seems quite out of date.

@etiennemartin-faire
Copy link

Once we have both of these changes it would be great if we could cut a new release of this library. We've grown quite dependant of SwiftyMocky and would like to avoid having to fork the repo. @spaluchiewicz Do you have any guidance on how to get changes published in this repo? It seems like its not moving very much, and we haven't seen an official release in 2 years.

@welshm
Copy link
Author

welshm commented Oct 9, 2024

@welshm Nice to see you working on this! I noticed that this change also requires two more changes. Some of the templates are hardcoded in swift. You can see that we'll need a similar change here as well:

return "\n\t\tget {\t\(staticModifier)invocations.append(.\(propertyCaseGetName)); return \(staticModifier)\(privatePrototypeName) ?? \(returnValue) }"

return "\n\t\tset {\t\(staticModifier)invocations.append(.\(propertyCaseSetName)(.value(newValue))); \(variable.isStatic ? "\(scope)." : "")\(privatePrototypeName) = newValue }"

I was going to re-create this PR from a Fork I created. If you could add it let me know. Our of curiosity as well. Did you manage to get the tests running locally? The project file seems quite out of date.

I did have the tests passing when I originally made this PR - no idea if they run or pass at this point.

I can add these changes (likely tomorrow) and see if tests still run

@welshm
Copy link
Author

welshm commented Oct 10, 2024

@welshm Nice to see you working on this! I noticed that this change also requires two more changes. Some of the templates are hardcoded in swift. You can see that we'll need a similar change here as well:

return "\n\t\tget {\t\(staticModifier)invocations.append(.\(propertyCaseGetName)); return \(staticModifier)\(privatePrototypeName) ?? \(returnValue) }"

return "\n\t\tset {\t\(staticModifier)invocations.append(.\(propertyCaseSetName)(.value(newValue))); \(variable.isStatic ? "\(scope)." : "")\(privatePrototypeName) = newValue }"

I was going to re-create this PR from a Fork I created. If you could add it let me know. Our of curiosity as well. Did you manage to get the tests running locally? The project file seems quite out of date.

I did have the tests passing when I originally made this PR - no idea if they run or pass at this point.

I can add these changes (likely tomorrow) and see if tests still run

I made the changes but I haven't tested them (and likely - I won't). I've moved to using a different mocking framework because this one seems on life support at best 😢

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.

Computed vars do not write to invocation array in a threadsafe way
6 participants