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

switch to use blackbox testing, move tests to separate module #35

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Nov 5, 2022

@thaJeztah thaJeztah force-pushed the blackbox_testing branch 3 times, most recently from d2ad08c to d2f5e41 Compare November 5, 2022 22:20
@thaJeztah thaJeztah marked this pull request as ready for review December 5, 2022 13:07
@thaJeztah
Copy link
Member Author

Okay, moved this out of draft for consideration.

Comment on lines -50 to +51
newSize := Winsize{Width: 200, Height: 200, x: winSize.x, y: winSize.y}
newSize := Winsize{Width: 200, Height: 200}
Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I think this was the bit I was still looking at. It's somewhat unclear why x and y aren't exported; effectively WinSize in this module is identical to golang.org/x/sys/unix.Winsize`;

Type in this module:

// Winsize represents the size of the terminal window.
type Winsize struct {
	Height uint16
	Width  uint16
	x      uint16
	y      uint16
}

And in golang.org/x/sys:

type Winsize struct {
	Row    uint16
	Col    uint16
	Xpixel uint16
	Ypixel uint16
}

In this module, we're converting the types between, so perhaps instead we should make it an alias (and deprecate the local type). Let me look at that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh! I recall now; for Windows, it's a different type, so we can make it an alias (but only on !windows)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm.. nevermind; field names are different 🤦

@thaJeztah thaJeztah marked this pull request as draft April 30, 2023 22:04
This allows us to remove the test-dependencies from the module itself

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the blackbox_testing branch from a6a5c39 to 3e71216 Compare May 2, 2023 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant