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

Unit tests that test location/size before BeginInit() and/or LayoutSubviews() are bogus #2449

Closed
tig opened this issue Mar 26, 2023 · 1 comment
Labels
breaking-change For PRs that introduces a breaking change (behavior or API) bug testing Issues related to testing v2 For discussions, issues, etc... relavant for v2

Comments

@tig
Copy link
Collaborator

tig commented Mar 26, 2023

E.g.

		[Fact]
		public void AutoSize_False_ResizeView_Is_Always_False ()
		{
			var label = new Label () { AutoSize = false };

			label.Text = "New text";

			Assert.False (label.AutoSize);
			Assert.Equal ("{X=0,Y=0,Width=0,Height=1}", label.Bounds.ToString ());
		}

It does not make sense to assume anything about the size or location of label (or any View) without LayoutSubviews() having been called on the view's superview.

This test passes in v1 because when Text is set, OnResizeNeeded (previously called ProcessResizeView) was always called:

		public virtual ustring Text {
			get => text;
			set {
				text = value;
				SetHotKey ();
				UpdateTextFormatterText ();
				ProcessResizeView ();
			}
		}

In v2, we need to use BeginInit/EndInit more cleanly, and code like the above will change to the following which breaks the above test.

		public virtual ustring Text {
			get => text;
			set {
				text = value;
				if (IsInitialized) {
					SetHotKey ();
					UpdateTextFormatterText ();
					OnResizeNeeded ();
				}
#if DEBUG
				if (text != null && string.IsNullOrEmpty (Id)) {
					Id = text.ToString ();
				}
#endif
			}
		}

I will be refactoring any such unit tests as part of #2360 like this:

		[Fact]
		public void AutoSize_False_ResizeView_Is_Always_False ()
		{
			var super = new View ();
			var label = new Label () { AutoSize = false };
			super.Add (label);

			label.Text = "New text";
			super.LayoutSubviews ();

			Assert.False (label.AutoSize);
			Assert.Equal ("{X=0,Y=0,Width=0,Height=1}", label.Bounds.ToString ());
		}
@tig tig added bug breaking-change For PRs that introduces a breaking change (behavior or API) testing Issues related to testing v2 For discussions, issues, etc... relavant for v2 labels Mar 26, 2023
@tig tig changed the title All unit tests that test location/size without callling superview.Add are bogus Unit tests that test location/size without callling superview.Add are bogus Mar 26, 2023
@tig tig moved this to 📋 Backlog in Terminal.Gui V2 Beta Mar 27, 2023
@tig tig moved this from 📋 Backlog to 🏗 In progress in Terminal.Gui V2 Beta Mar 27, 2023
@tig tig moved this from 🏗 In progress to 👀 In review in Terminal.Gui V2 Beta Apr 13, 2023
@tig
Copy link
Collaborator Author

tig commented Apr 16, 2023

This is largely addressed. There may be a few stragglers. If an old unit test is failing and you can't figure out why, consider that you're accessing View properties that should only be accessed after BeginEnd() and/or LayoutSubViews() has been called.

@tig tig closed this as completed Apr 16, 2023
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Terminal.Gui V2 Beta Apr 16, 2023
@tig tig changed the title Unit tests that test location/size without callling superview.Add are bogus Unit tests that test location/size before BeginInit() and/or LayoutSubviews() are bogus Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change For PRs that introduces a breaking change (behavior or API) bug testing Issues related to testing v2 For discussions, issues, etc... relavant for v2
Projects
No open projects
Status: ✅ Done
Development

No branches or pull requests

1 participant