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.

Sign up for GitHub

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

Unskipping tea tests and adding protection around possible nil models #8053

Merged
merged 1 commit into from
Jan 27, 2025
Merged

Unskipping tea tests and adding protection around possible nil models #8053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion pkg/cli/cmd/deploy/deploy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,6 @@ func Test_Run(t *testing.T) {
})

t.Run("Deployment with missing parameters", func(t *testing.T) {
//t.Skip()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

Expand Down
42 changes: 21 additions & 21 deletions pkg/cli/cmd/radinit/display_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ var (
)

func Test_summaryModel(t *testing.T) {
t.Skip("Test is flaky. See: https://github.com/radius-project/radius/issues/8044")

waitForRender := func(t *testing.T, reader io.Reader) string {
normalized := ""
teatest.WaitFor(t, reader, func(bts []byte) bool {
Expand All @@ -47,16 +45,6 @@ func Test_summaryModel(t *testing.T) {
return normalized
}

waitForEmpty := func(t *testing.T, reader io.Reader) string {
normalized := ""
teatest.WaitFor(t, reader, func(bts []byte) bool {
normalized = stripansi.Strip(strings.ReplaceAll(string(bts), "\r\n", "\n"))
return !strings.Contains(normalized, strings.Trim(summaryFooter, "\n"))
}, teatest.WithDuration(waitTimeout))

return normalized
}

Comment on lines -50 to -59
Copy link
Contributor

@vishwahiremat vishwahiremat Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I am trying to understand why is this part of the code not needed anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tm.Quit is taking care of this part now.

resultTest := func(t *testing.T, expected summaryResult, key tea.KeyType) {
options := initOptions{}
model := &summaryModel{
Expand All @@ -66,12 +54,18 @@ func Test_summaryModel(t *testing.T) {

waitForRender(t, tm.Output())

// Press ENTER
tm.Send(tea.KeyMsg{Type: key})
// Press the given key
tm.Send(tea.KeyMsg{
Type: key,
})

// Wait for final render and exit.
tm.WaitFinished(t, teatest.WithFinalTimeout(waitTimeout))
waitForEmpty(t, tm.FinalOutput(t))
if err := tm.Quit(); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please see example tests from the original repo: https://github.com/charmbracelet/x/blob/20117e9c8cd5ad229645f1bca3422b7e4110c96c/exp/teatest/app_test.go#L34. We can discuss about them.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a different test though right? We're now explicitly telling the model to quit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment below this line. I turns out that the FinalModel only returns once the program has finished running or when it times out. Please see the official repo doc. That is why we call tm.Quit() before tm.FinalModel().

Is there a reason why we shouldn't call the quit function? I believe if we don't call it, we can't get the final model unless we wait for it to timeout.

Anything I may be missing here? cc/ @vishwahiremat

t.Fatal(err)
}

// FinalModel only returns once the program has finished running or when it times out.
// Please see: https://github.com/charmbracelet/x/blob/20117e9c8cd5ad229645f1bca3422b7e4110c96c/exp/teatest/teatest.go#L220.
// That is why we call tm.Quit() before tm.FinalModel().
model = tm.FinalModel(t).(*summaryModel)
require.Equal(t, expected, model.result)
}
Expand All @@ -98,11 +92,17 @@ func Test_summaryModel(t *testing.T) {
assert.Equal(t, expected, output)

// Press ENTER
tm.Send(tea.KeyMsg{Type: tea.KeyEnter})
tm.Send(tea.KeyMsg{
Type: tea.KeyEnter,
})

if err := tm.Quit(); err != nil {
t.Fatal(err)
}

// Wait for final render and exit.
tm.WaitFinished(t, teatest.WithFinalTimeout(waitTimeout))
waitForEmpty(t, tm.FinalOutput(t))
// FinalModel only returns once the program has finished running or when it times out.
// Please see: https://github.com/charmbracelet/x/blob/20117e9c8cd5ad229645f1bca3422b7e4110c96c/exp/teatest/teatest.go#L220.
// That is why we call tm.Quit() before tm.FinalModel().
model = tm.FinalModel(t).(*summaryModel)
assert.Equal(t, summaryResult(resultConfimed), model.result)
}
Expand Down
61 changes: 46 additions & 15 deletions pkg/cli/prompt/text/text_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ func validateNewTextModel(t *testing.T, model *Model, options *TextModelOptions)
}

func Test_E2E(t *testing.T) {

const expectedPrompt = "\r" + testPrompt + "\n" +
"\n" +
"> " + testPlaceholder + "\n" +
Expand Down Expand Up @@ -122,35 +121,67 @@ func Test_E2E(t *testing.T) {
})

t.Run("confirm default", func(t *testing.T) {
t.Skip("This test is intermittently failing in linux_amd64: https://github.com/radius-project/radius/issues/7670")
tm := setup(t)
tm.Send(tea.KeyMsg{
Type: tea.KeyEnter,
})

if err := tm.Quit(); err != nil {
t.Fatal(err)
}

tm.Send(tea.KeyMsg{Type: tea.KeyEnter})
// FinalModel only returns once the program has finished running or when it times out.
// Please see: https://github.com/charmbracelet/x/blob/20117e9c8cd5ad229645f1bca3422b7e4110c96c/exp/teatest/teatest.go#L220.
// That is why we call tm.Quit() before tm.FinalModel().
model, ok := tm.FinalModel(t).(Model)
require.True(t, ok, "Final model should be of type Model")

require.True(t, tm.FinalModel(t).(Model).valueEntered)
require.False(t, tm.FinalModel(t).(Model).Quitting)
require.Equal(t, defaultText, tm.FinalModel(t).(Model).GetValue())
require.True(t, model.valueEntered)
require.False(t, model.Quitting)
require.Equal(t, defaultText, model.GetValue())
})

t.Run("confirm value", func(t *testing.T) {
const userInputText = "abcd"
tm := setup(t)

tm.Type(userInputText)
tm.Send(tea.KeyMsg{Type: tea.KeyEnter})
tm.Send(tea.KeyMsg{
Type: tea.KeyEnter,
})

require.True(t, tm.FinalModel(t).(Model).valueEntered)
require.False(t, tm.FinalModel(t).(Model).Quitting)
require.Equal(t, userInputText, tm.FinalModel(t).(Model).GetValue())
if err := tm.Quit(); err != nil {
t.Fatal(err)
}

// FinalModel only returns once the program has finished running or when it times out.
// Please see: https://github.com/charmbracelet/x/blob/20117e9c8cd5ad229645f1bca3422b7e4110c96c/exp/teatest/teatest.go#L220.
// That is why we call tm.Quit() before tm.FinalModel().
model, ok := tm.FinalModel(t).(Model)
require.True(t, ok, "Final model should be of type Model")

require.True(t, model.valueEntered)
require.False(t, model.Quitting)
require.Equal(t, userInputText, model.GetValue())
})

t.Run("cancel", func(t *testing.T) {
tm := setup(t)
tm.Send(tea.KeyMsg{
Type: tea.KeyCtrlC,
})

if err := tm.Quit(); err != nil {
t.Fatal(err)
}

tm.Send(tea.KeyMsg{Type: tea.KeyCtrlC})
// FinalModel only returns once the program has finished running or when it times out.
// Please see: https://github.com/charmbracelet/x/blob/20117e9c8cd5ad229645f1bca3422b7e4110c96c/exp/teatest/teatest.go#L220.
// That is why we call tm.Quit() before tm.FinalModel().
model, ok := tm.FinalModel(t).(Model)
require.True(t, ok, "Final model should be of type Model")

require.False(t, tm.FinalModel(t).(Model).valueEntered)
require.True(t, tm.FinalModel(t).(Model).Quitting)
require.Equal(t, defaultText, tm.FinalModel(t).(Model).GetValue())
require.False(t, model.valueEntered)
require.True(t, model.Quitting)
require.Equal(t, defaultText, model.GetValue())
})
}
Loading