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 problem report logs being duplicated #6235

Merged
merged 1 commit into from
May 13, 2024

Conversation

rablador
Copy link
Contributor

@rablador rablador commented May 8, 2024

Some problem reports get duplicated when displaying logs in the problem report view.


This change is Reviewable

@rablador rablador added bug iOS Issues related to iOS labels May 8, 2024
@rablador rablador requested review from buggmagnet and mojganii May 8, 2024 15:54
@rablador rablador self-assigned this May 8, 2024
Copy link

linear bot commented May 8, 2024

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.

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


ios/Shared/ApplicationConfiguration.swift line 41 at r1 (raw file):

            let pathIsLog = path.split(separator: ".").last == "log"
            // Pattern should be either "net.mullvad.MullvadVPN:PacketTunnel_" or "net.mullvad.MullvadVPN_".
            let pathBelongsToTarget = path.contains("\(target.bundleIdentifier)_")

I think we should write a test for that.
Also there's a small typo, it should be net.mullvad.MullvadVPN.PacketTunnel_ (dot instead of colon)

@rablador rablador force-pushed the log-file-size-limit-does-not-work-ios-681 branch 2 times, most recently from d80da60 to df61114 Compare May 10, 2024 14:01
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: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @buggmagnet)


ios/Shared/ApplicationConfiguration.swift line 41 at r1 (raw file):

Previously, buggmagnet wrote…

I think we should write a test for that.
Also there's a small typo, it should be net.mullvad.MullvadVPN.PacketTunnel_ (dot instead of colon)

Done.

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.

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


ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift line 18 at r2 (raw file):

    override func setUpWithError() throws {
        try? fileManager.createDirectory(

We shouldn't try?, if the test directory cannot be created, the tests should fail.

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 @buggmagnet)


ios/MullvadVPNTests/MullvadLogging/LoggingTests.swift line 18 at r2 (raw file):

Previously, buggmagnet wrote…

We shouldn't try?, if the test directory cannot be created, the tests should fail.

Done.

@rablador rablador force-pushed the log-file-size-limit-does-not-work-ios-681 branch from df61114 to 963337e Compare May 13, 2024 07:57
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:

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

@buggmagnet buggmagnet force-pushed the log-file-size-limit-does-not-work-ios-681 branch from 963337e to 168bd35 Compare May 13, 2024 08:17
@buggmagnet buggmagnet merged commit 4380b7a into main May 13, 2024
7 checks passed
@buggmagnet buggmagnet deleted the log-file-size-limit-does-not-work-ios-681 branch May 13, 2024 08:19
Copy link

🚨 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.

2 participants