Dialog: Revert #8964 (Fix possible deadlock)#9030
Conversation
Reverts MudBlazor#8964 Caused timing issues
github-actions
bot
added
the
bug
|
@danielchalmers what does that mean Ah nvm, I got it, it means it might deadlock again after the revert. |
henon
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #9030 +/- ##
==========================================
+ Coverage 89.82% 90.62% +0.79%
==========================================
Files 412 398 -14
Lines 11878 12375 +497
Branches 2364 2403 +39
==========================================
+ Hits 10670 11215 +545
+ Misses 681 621 -60
- Partials 527 539 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I will investigate this later with daniels project etc |
@ScarletKuro I don't think I translated the tests properly because they are working on MudBlazor. Will try to get them failing later (lol). But yeah in the meantime my project still fails with p3 package. Requires MAUI |
|
@ScarletKuro Struggling to get the test failing on the MB test suite despite the fact that it decisively fails on my project with an update from p2 to p3. |
|
I haven't worked with bUnit's Note test[Fact]
public void Note()
{
var category = new DataPointCategory
{
Guid = Guid.NewGuid(),
Type = PointType.Note,
};
var day = Day.Create(new(2024, 01, 01));
var point = DataPoint.Create(day, category);
point.Text = "Hello world";
var layout = Render(
@<MainLayout>
<Body>
<DataPointView Point="point" />
</Body>
</MainLayout>);
var cut = () => layout.FindComponent<DataPointView>();
cut().Find(".mud-link").Click();
var noteEditor = () => layout.FindComponent<EditNoteDialog>();
noteEditor().Find("textarea").Input("123");
layout.Find(".submit-button").Click();
point.Text.Should().Be("123");
cut().Find(".mud-link").Click();
noteEditor().Find("textarea").Input("EXTRA TEXT THAT WILL BE DISCARDED");
layout.Find(".cancel-button").Click();
point.Text.Should().Be("123");
}and MedicationEditDose test[Fact]
public void MedicationEditDose()
{
var category = new DataPointCategory
{
Guid = Guid.NewGuid(),
Type = PointType.Medication,
MedicationUnit = "oz",
MedicationDose = 11,
};
var day = Day.Create(new(2024, 01, 01));
var point = DataPoint.Create(day, category);
point.Text = "Hello world";
point.MedicationDose = 22;
var layout = Render(
@<MainLayout>
<Body>
<DataPointView Point="point" />
</Body>
</MainLayout>);
var cut = () => layout.FindComponent<DataPointView>();
// Initial state.
point.Bool.Should().Be(null);
cut().FindAll(".mud-toggle-item-selected").Count.Should().Be(0);
// Change dose to empty via dialog.
cut().Find(".mud-link").Click();
var doseEditor = () => layout.FindComponent<EditDoseDialog>();
doseEditor().Find("input").Input("");
layout.Find(".submit-button").Click();
// Empty dose resets to default and selects No.
point.MedicationDose.Should().Be(point.Category.MedicationDose);
cut().FindAll(".mud-toggle-item")[0].ClassList.Should().Contain("mud-toggle-item-selected");
// Change dose via dialog.
cut().Find(".mud-link").Click();
doseEditor().Find("input").Input("99");
layout.Find(".submit-button").Click();
// Submitting dose is the same as Yes.
point.MedicationDose.Should().Be(99);
cut().FindAll(".mud-toggle-item")[1].ClassList.Should().Contain("mud-toggle-item-selected");
// Cancel dialog.
cut().Find(".mud-link").Click();
doseEditor().Find("input").Input("88");
layout.Find(".cancel-button").Click();
// Same as before
point.MedicationDose.Should().Be(99);
cut().FindAll(".mud-toggle-item")[1].ClassList.Should().Contain("mud-toggle-item-selected");
}It works as expected. |
|
By the way, if you ask why it worked before, I don't know, as it requires understanding how bUnit works internally. This isn't the first time this has happened. For example, when a Task method becomes truly asynchronous: |
|
Thanks for all the help @ScarletKuro. I opened a discussion with bUnit to confirm bUnit-dev/bUnit#1470 |
|
@danielchalmers another similar thing happened when you asked for this change: #8203. It doesn't have anything to do with the async aspect, but rather with the render counts. However, we also had to replace all |
@ScarletKuro Great, I'm trying to learn more about this area so thank you again! |
|
Ah I might be wrong. I was running the test individually so it works, but it is failing when you run all the tests at same time and it does with my changes too |
ScarletKuro
changed the title
ScarletKuro
changed the title
|
I'm reverting this. The reason is that the suggested test improvement turned out to not work. While it's still weird that the tests pass when you run them individually but fail when you run them in batch (seems like it's some xUnit thing), my time is becoming more limited each day and I don't want to invest too much time in finding the root cause. However, I also don't want people's tests to be broken. This change was made to align the code with Blazored/Modal, and it was something that Microsoft recommended in their async guide. But in fact, nobody really complained about any deadlock in the dialog. Additionally, in the future, the dialog is most likely going to undergo some kind of rework. |
danielchalmers
added
regression
3 participants
Description
Reverts #8964
Caused timing issues in my app's tests. I tried to replicate it in the MudBlazor tests but it won't compile.
How Has This Been Tested?
Type of Changes
Checklist
dev).