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

Fix failing log rotation test #6936

Merged

Conversation

rablador
Copy link
Contributor

@rablador rablador commented Oct 7, 2024

The test testRotatingActiveLogWhenSizeLimitIsExceeded has been failing on the CI for a while. This PR enables forced synchronization of the asynchronous writes in that tests and similar to it.


This change is Reviewable

@rablador rablador added bug iOS Issues related to iOS labels Oct 7, 2024
@rablador rablador self-assigned this Oct 7, 2024
Copy link

linear bot commented Oct 7, 2024

Copy link
Collaborator

@pinkisemils pinkisemils left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @rablador)


ios/MullvadLogging/LogFileOutputStream.swift line 78 at r1 (raw file):

    }

    /// Waits for write operations to finish by issuing a synchronous closure.

Could do with explaining it should only be used in testing.


ios/MullvadVPN.xcodeproj/project.pbxproj line 2500 at r1 (raw file):

			children = (
				44B02E3A2BC5732D008EDF34 /* LoggingTests.swift */,
				7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */,

Thanks, Xcode.

@rablador rablador force-pushed the fix-testrotatingactivelogwhensizelimitisexceeded-ios-844 branch from e23480f to 20ae676 Compare October 7, 2024 11:17
Copy link
Contributor Author

@rablador rablador left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pinkisemils)


ios/MullvadLogging/LogFileOutputStream.swift line 78 at r1 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

Could do with explaining it should only be used in testing.

Done.


ios/MullvadVPN.xcodeproj/project.pbxproj line 2500 at r1 (raw file):

Previously, pinkisemils (Emīls Piņķis) wrote…

Thanks, Xcode.

No, I did that. We keep files ordered alphabetically, and these werent.

Copy link
Collaborator

@pinkisemils pinkisemils left a comment

Choose a reason for hiding this comment

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

Reviewable status: 3 of 4 files reviewed, all discussions resolved


ios/MullvadVPN.xcodeproj/project.pbxproj line 2500 at r1 (raw file):

Previously, rablador (Jon Petersson) wrote…

No, I did that. We keep files ordered alphabetically, and these werent.

Oh, cool.

Copy link
Collaborator

@pinkisemils pinkisemils left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Contributor

@buggmagnet buggmagnet left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@buggmagnet buggmagnet force-pushed the fix-testrotatingactivelogwhensizelimitisexceeded-ios-844 branch from 20ae676 to 9f866a0 Compare October 8, 2024 09:23
@buggmagnet buggmagnet merged commit adbd44a into main Oct 8, 2024
10 of 11 checks passed
@buggmagnet buggmagnet deleted the fix-testrotatingactivelogwhensizelimitisexceeded-ios-844 branch October 8, 2024 10:51
Copy link

github-actions bot commented Oct 8, 2024

🚨 End to end tests failed. Please check the failed workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants