Skip to content

Commit

Permalink
Add display to the engine tests (#16050)
Browse files Browse the repository at this point in the history
We want to add more test coverage to the display code. The best way to
do that is to add it to the engine tests, that already cover most of the
pulumi functionality.

It's probably not really possible to review all of the output, but at
least it gives us a baseline, which we can work with.

There's a couple of tests that are flaky for reasons I don't quite
understand yet. I marked them as to skip and we can look at them later.
I'd rather get in the baseline tests sooner, rather than spending a
bunch of time looking at that. The output differences also seem very
minor, so not super concerning.

The biggest remaining issue is that this doesn't interact well with the
Chdir we're doing in the engine. We could either pass the CWD through,
or just try to get rid of that Chdir. So this should only be merged
after #15607.

I've tried to split this into a few commits, separating out adding the
testdata, so it's hopefully a little easier to review, even though the
PR is still quite large.

One other thing to note is that we're comparing that the output has all
the same lines, and not that it is exactly the same. Because of how the
engine is implemented, there's a bunch of race conditions otherwise,
that would make us have to skip a bunch of tests, just because e.g.
resource A is sometimes deleted before resource B and sometimes it's the
other way around.

The biggest downside of that is that running with `PULUMI_ACCEPT` will
produce a diff even when there are no changes. Hopefully we won't have
to run that way too often though, so it might not be a huge issue?

---------

Co-authored-by: Fraser Waters <[email protected]>
  • Loading branch information
tgummerer and Frassle committed May 13, 2024
1 parent 48191af commit fc10da3
Show file tree
Hide file tree
Showing 4,020 changed files with 54,079 additions and 484 deletions.
The diff you're trying to view is too large. We only load the first 3000 changed files.
2 changes: 1 addition & 1 deletion pkg/backend/display/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type Options struct {

// testing-only options
term terminal.Terminal
deterministicOutput bool
DeterministicOutput bool
}

func (opts Options) WithIsInteractive(isInteractive bool) Options {
Expand Down
50 changes: 47 additions & 3 deletions pkg/backend/display/progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func ShowProgressEvents(op string, action apitype.UpdateKind, stack tokens.Stack
renderer.initializeDisplay(display)

ticker := time.NewTicker(1 * time.Second)
if opts.deterministicOutput {
if opts.DeterministicOutput {
ticker.Stop()
}
display.processEvents(ticker, events)
Expand Down Expand Up @@ -472,6 +472,25 @@ func (display *ProgressDisplay) processEndSteps() {
// Now print out all those rows that were in progress. They will now be 'done'
// since the display was marked 'done'.
if !display.isTerminal {
if display.opts.DeterministicOutput {
sort.Slice(inProgressRows, func(i, j int) bool {
if inProgressRows[i].Step().Op == "same" && inProgressRows[i].Step().URN == "" {
// This is the root stack event. Always sort it last
return false
}
if inProgressRows[j].Step().Op == "same" && inProgressRows[j].Step().URN == "" {
// This is the root stack event. Always sort it last
return true
}
if inProgressRows[i].Step().Res == nil {
return false
}
if inProgressRows[j].Step().Res == nil {
return true
}
return inProgressRows[i].Step().Res.URN < inProgressRows[j].Step().Res.URN
})
}
for _, v := range inProgressRows {
display.renderer.rowUpdated(v)
}
Expand Down Expand Up @@ -510,7 +529,32 @@ func (display *ProgressDisplay) printDiagnostics() bool {
// Since we display diagnostic information eagerly, we need to keep track of the first
// time we wrote some output so we don't inadvertently print the header twice.
wroteDiagnosticHeader := false

eventRows := make([]ResourceRow, 0, len(display.eventUrnToResourceRow))
for _, row := range display.eventUrnToResourceRow {
eventRows = append(eventRows, row)
}
if display.opts.DeterministicOutput {
sort.Slice(eventRows, func(i, j int) bool {
if eventRows[i].Step().Op == "same" && eventRows[i].Step().URN == "" {
// This is the root stack event. Always sort it last
return false
}
if eventRows[j].Step().Op == "same" && eventRows[j].Step().URN == "" {
// This is the root stack event. Always sort it last
return true
}
if eventRows[i].Step().Res == nil {
return false
}
if eventRows[j].Step().Res == nil {
return true
}
return eventRows[i].Step().Res.URN < eventRows[j].Step().Res.URN
})
}

for _, row := range eventRows {
// The header for the diagnogistics grouped by resource, e.g. "aws:apigateway:RestApi (accountsApi):"
wroteResourceHeader := false

Expand Down Expand Up @@ -1186,7 +1230,7 @@ func (display *ProgressDisplay) getStepDoneDescription(step engine.StepEventMeta
return ""
}
}
if op == deploy.OpSame || display.opts.deterministicOutput || display.opts.SuppressTimings {
if op == deploy.OpSame || display.opts.DeterministicOutput || display.opts.SuppressTimings {
return opText
}

Expand Down Expand Up @@ -1365,7 +1409,7 @@ func (display *ProgressDisplay) getStepInProgressDescription(step engine.StepEve
return ""
}

if op == deploy.OpSame || display.opts.deterministicOutput || display.opts.SuppressTimings {
if op == deploy.OpSame || display.opts.DeterministicOutput || display.opts.SuppressTimings {
return opText
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/backend/display/progress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func testProgressEvents(t *testing.T, path string, accept, interactive bool, wid
Stdout: &stdout,
Stderr: &stderr,
term: terminal.NewMockTerminal(&stdout, width, height, true),
deterministicOutput: true,
DeterministicOutput: true,
}, false)

for _, e := range events {
Expand Down Expand Up @@ -271,7 +271,7 @@ func TestProgressPolicyPacks(t *testing.T) {
Stdout: &stdout,
Stderr: &stderr,
term: terminal.NewMockTerminal(&stdout, 80, 24, true),
deterministicOutput: true,
DeterministicOutput: true,
}, false)

// Send policy pack event to the channel
Expand Down
4 changes: 2 additions & 2 deletions pkg/backend/display/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func newInteractiveRenderer(term terminal.Terminal, permalink string, opts Optio
keys: make(chan string),
closed: make(chan bool),
}
if opts.deterministicOutput {
if opts.DeterministicOutput {
r.ticker.Stop()
}
go r.handleEvents()
Expand Down Expand Up @@ -189,7 +189,7 @@ func (r *treeRenderer) markDirty() {
}

r.dirty = true
if r.opts.deterministicOutput {
if r.opts.DeterministicOutput {
r.frame(true, false)
}
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/backend/httpstate/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,9 @@ func generateSnapshots(t testing.TB, r *rand.Rand, resourceCount, resourcePayloa

var journalEntries engine.JournalEntries
p := &lifecycletest.TestPlan{
Options: lifecycletest.TestUpdateOptions{HostF: hostF},
// This test generates big amounts of data so the event streams that would need to be
// checked in get too big. Skip them instead.
Options: lifecycletest.TestUpdateOptions{T: t, HostF: hostF, SkipDisplayTests: true},
Steps: []lifecycletest.TestStep{
{
Op: engine.Update,
Expand Down

0 comments on commit fc10da3

Please sign in to comment.