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 some crashing tests #5261

Merged
merged 18 commits into from
Apr 8, 2022
Merged

Fix some crashing tests #5261

merged 18 commits into from
Apr 8, 2022

Conversation

mattleibow
Copy link
Member

@mattleibow mattleibow commented Mar 12, 2022

Description of Change

One test seemed to go bad after #4933 and that was because the PR did not catch the bad test. This is because of 2 reasons:

  • We disabled the warnings in the IDE
    We have disabled the warnings because we have an API that we use that the analyzer did not know about. We are in the process of fixing this, but we went too far and turned off the warnings instead of downgrading to suggestions.
  • We didn't actually test all the android levels
    We don't because there are a lot to test, but maybe we should have the latest and the oldest in the PR just to be safe. The runs are pretty quick on the accelerated bots, so this won't add much time.

Issues Fixed

A crashing test and total lack of warnings.

NOTE

I was unable to fix the tests on older Android devices, so I disabled them there and opened this issue: #5903

.editorconfig Outdated Show resolved Hide resolved
eng/pipelines/device-tests.yml Show resolved Hide resolved
@mattleibow
Copy link
Member Author

lol:

9>CSC : error AD0001: Analyzer 'Microsoft.NetCore.Analyzers.InteropServices.PlatformCompatibilityAnalyzer' threw an exception of type 'System.NullReferenceException' with message 'Object reference not set to an instance of an object.'. [D:\a_work\1\s\src\Essentials\src\Essentials.csproj]

@mattleibow mattleibow changed the title Fix some crashing tests Fix some crashing tests & localization Mar 14, 2022
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s), but failed to run 1 pipeline(s).

@Redth Redth added this to the 6.0.300-rc.1 milestone Mar 21, 2022
@mattleibow
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

@PureWeen PureWeen self-requested a review March 23, 2022 20:46
@PureWeen PureWeen dismissed their stale review March 23, 2022 20:51

Fixed Tests

@rmarinho
Copy link
Member

@mattleibow mattleibow added the area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions label Mar 25, 2022
@mattleibow
Copy link
Member Author

For some reason, the same font is loaded but with different handles on older devices. It is not default font, so it seems that it just loaded 2 of the same fonts n memory...

@Redth Redth modified the milestones: 6.0.300-rc.1, 6.0.300-rc.2 Mar 26, 2022
# Conflicts:
#	src/Core/src/Platform/Android/PlatformVersion.cs
@mattleibow mattleibow changed the title Fix some crashing tests & localization Fix some crashing tests Apr 8, 2022
@rmarinho rmarinho merged commit e171d1f into main Apr 8, 2022
@rmarinho rmarinho deleted the dev/fix-tests branch April 8, 2022 13:05
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-infrastructure CI, Maestro / Coherency, upstream dependencies/versions t/housekeeping ♻︎
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants