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

web: remove test and non-test default resource grouping #4781

Merged
merged 1 commit into from
Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
120 changes: 5 additions & 115 deletions web/src/OverviewSidebarOptions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,13 @@ import {
import { accessorsForTesting, tiltfileKeyContext } from "./LocalStorage"
import {
TestsWithErrors,
TwoResources,
TwoResourcesTwoTests,
} from "./OverviewResourceSidebar.stories"
import {
AlertsOnTopToggle,
ClearResourceNameFilterButton,
FilterOptionList,
OverviewSidebarOptions,
ResourceNameFilterTextField,
TestsHiddenToggle,
TestsOnlyToggle,
} from "./OverviewSidebarOptions"
import SidebarItemView from "./SidebarItemView"
import SidebarResources, {
Expand All @@ -36,8 +32,6 @@ const sidebarOptionsAccessor = accessorsForTesting<SidebarOptions>(
export function assertSidebarItemsAndOptions(
root: ReactWrapper,
names: string[],
expectTestsHidden: boolean,
expectTestsOnly: boolean,
expectAlertsOnTop: boolean,
expectedResourceNameFilter?: string
) {
Expand All @@ -53,12 +47,6 @@ export function assertSidebarItemsAndOptions(

let optSetter = sidebar.find(OverviewSidebarOptions)
expect(optSetter).toHaveLength(1)
expect(optSetter.find(TestsHiddenToggle).hasClass("is-enabled")).toEqual(
expectTestsHidden
)
expect(optSetter.find(TestsOnlyToggle).hasClass("is-enabled")).toEqual(
expectTestsOnly
)
expect(optSetter.find(AlertsOnTopToggle).hasClass("is-enabled")).toEqual(
expectAlertsOnTop
)
Expand All @@ -69,13 +57,6 @@ export function assertSidebarItemsAndOptions(
}
}

function clickTestsHiddenControl(root: ReactWrapper) {
root.find(TestsHiddenToggle).simulate("click")
}
function clickTestsOnlyControl(root: ReactWrapper) {
root.find(TestsOnlyToggle).simulate("click")
}

const allNames = ["(Tiltfile)", "vigoda", "snack", "beep", "boop"]

describe("overview sidebar options", () => {
Expand All @@ -92,91 +73,7 @@ describe("overview sidebar options", () => {

it("shows all resources by default", () => {
const root = mount(TwoResourcesTwoTests())
assertSidebarItemsAndOptions(root, allNames, false, false, false)
})

it("hides tests when TestsHidden enabled", () => {
const root = mount(TwoResourcesTwoTests())
assertSidebarItemsAndOptions(root, allNames, false, false, false)

clickTestsHiddenControl(root)
assertSidebarItemsAndOptions(
root,
["(Tiltfile)", "vigoda", "snack"],
true,
false,
false
)

expectIncrs({
name: "ui.web.testsHiddenToggle",
tags: { action: "click", newTestsHiddenState: "true" },
})

// re-check and make sure everything is visible
clickTestsHiddenControl(root)
assertSidebarItemsAndOptions(root, allNames, false, false, false)
})

it("shows only tests when TestsOnly enabled", () => {
const root = mount(TwoResourcesTwoTests())
assertSidebarItemsAndOptions(root, allNames, false, false, false)

clickTestsOnlyControl(root)
assertSidebarItemsAndOptions(root, ["beep", "boop"], false, true, false)

expectIncrs({
name: "ui.web.testsOnlyToggle",
tags: { action: "click", newTestsOnlyState: "true" },
})

// re-check and make sure tests are visible
clickTestsOnlyControl(root)
assertSidebarItemsAndOptions(root, allNames, false, false, false)
})

it("only one filter option can be enabled at once", () => {
const root = mount(TwoResourcesTwoTests())
assertSidebarItemsAndOptions(root, allNames, false, false, false)

clickTestsHiddenControl(root)
clickTestsOnlyControl(root)
assertSidebarItemsAndOptions(root, ["beep", "boop"], false, true, false)

// Make sure it works in the other direction too
clickTestsHiddenControl(root)
assertSidebarItemsAndOptions(
root,
["(Tiltfile)", "vigoda", "snack"],
true,
false,
false
)
})

it("doesn't show filter options if no tests present", () => {
const root = mount(TwoResources())

let sidebar = root.find(SidebarResources)
expect(sidebar).toHaveLength(1)

let filters = sidebar.find(FilterOptionList)
expect(filters).toHaveLength(0)
})

it("shows filter options when no tests are present if filter options are non-default", () => {
sidebarOptionsAccessor.set({ ...defaultOptions, testsHidden: true })
const root = mount(
<tiltfileKeyContext.Provider value="test">
{TwoResources()}
</tiltfileKeyContext.Provider>
)

let sidebar = root.find(SidebarResources)
expect(sidebar).toHaveLength(1)

let filters = sidebar.find(FilterOptionList)
expect(filters).toHaveLength(1)
assertSidebarItemsAndOptions(root, allNames, false)
})

it("applies the name filter", () => {
Expand All @@ -195,8 +92,6 @@ describe("overview sidebar options", () => {
assertSidebarItemsAndOptions(
root,
["beep", "boop"],
defaultOptions.testsHidden,
defaultOptions.testsOnly,
defaultOptions.alertsOnTop,
"B p"
)
Expand Down Expand Up @@ -225,11 +120,7 @@ describe("overview sidebar options", () => {

it("reports analytics, debounced, when search bar edited", () => {
const root = mount(
<OverviewSidebarOptions
options={defaultOptions}
setOptions={() => {}}
showFilters={true}
/>
<OverviewSidebarOptions options={defaultOptions} setOptions={() => {}} />
)
const tf = root.find(ResourceNameFilterTextField)
// two changes in rapid succession should result in only one analytics event
Expand All @@ -245,7 +136,6 @@ describe("overview sidebar options", () => {
<OverviewSidebarOptions
options={{ ...defaultOptions, resourceNameFilter: "foo" }}
setOptions={() => {}}
showFilters={true}
/>
)
const button = root.find(ClearResourceNameFilterButton)
Expand Down Expand Up @@ -283,13 +173,13 @@ it("toggles/untoggles Alerts On Top sorting when button clicked", () => {
"test_5",
"test_7",
]
assertSidebarItemsAndOptions(root, origOrder, false, false, false)
assertSidebarItemsAndOptions(root, origOrder, false)

let aotToggle = root.find(AlertsOnTopToggle)
aotToggle.simulate("click")

assertSidebarItemsAndOptions(root, alertsOnTopOrder, false, false, true)
assertSidebarItemsAndOptions(root, alertsOnTopOrder, true)

aotToggle.simulate("click")
assertSidebarItemsAndOptions(root, origOrder, false, false, false)
assertSidebarItemsAndOptions(root, origOrder, false)
})
62 changes: 2 additions & 60 deletions web/src/OverviewSidebarOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ export const ClearResourceNameFilterButton = styled(InstrumentedButton)`
`

type OverviewSidebarOptionsProps = {
showFilters: boolean
options: SidebarOptions
setOptions: Dispatch<SetStateAction<SidebarOptions>>
}
Expand All @@ -141,30 +140,6 @@ function setAlertsOnTop(
})
}

function toggleTestsOnly(props: OverviewSidebarOptionsProps) {
props.setOptions((prevOptions) => {
// Always set the option you're not currently toggling to 'false', because both
// of these settings cannot be 'true' at the same time
return {
...prevOptions,
testsHidden: false,
testsOnly: !prevOptions.testsOnly,
}
})
}

function toggleTestsHidden(props: OverviewSidebarOptionsProps) {
props.setOptions((prevOptions) => {
// Always set the option you're not currently toggling to 'false', because both
// of these settings cannot be 'true' at the same time
return {
...prevOptions,
testsHidden: !prevOptions.testsHidden,
testsOnly: false,
}
})
}

function setResourceNameFilter(
newValue: string,
props: OverviewSidebarOptionsProps
Expand All @@ -177,36 +152,6 @@ function setResourceNameFilter(
})
}

function filterOptions(props: OverviewSidebarOptionsProps) {
return (
<FilterOptionList>
Tests:
<ResourceFilterSegmentedControls>
<TestsHiddenToggle
className={props.options.testsHidden ? "is-enabled" : ""}
onClick={(e) => toggleTestsHidden(props)}
analyticsName="ui.web.testsHiddenToggle"
analyticsTags={{
newTestsHiddenState: (!props.options.testsHidden).toString(),
}}
>
Hidden
</TestsHiddenToggle>
<TestsOnlyToggle
className={props.options.testsOnly ? "is-enabled" : ""}
onClick={(e) => toggleTestsOnly(props)}
analyticsName="ui.web.testsOnlyToggle"
analyticsTags={{
newTestsOnlyState: (!props.options.testsOnly).toString(),
}}
>
Only
</TestsOnlyToggle>
</ResourceFilterSegmentedControls>
</FilterOptionList>
)
}

// debounce so we don't send for every single keypress
let incrResourceNameFilterEdit = debounce(() => {
incr("ui.web.resourceNameFilter", { action: "edit" })
Expand Down Expand Up @@ -260,13 +205,10 @@ export function OverviewSidebarOptions(
): JSX.Element {
return (
<OverviewSidebarOptionsRoot>
<OverviewSidebarOptionsButtonsRoot
className={!props.showFilters ? "is-filterButtonsHidden" : ""}
>
{props.showFilters ? filterOptions(props) : null}
<OverviewSidebarOptionsButtonsRoot className="is-filterButtonsHidden">
<AlertsOnTopToggle
className={props.options.alertsOnTop ? "is-enabled" : ""}
onClick={(e) => setAlertsOnTop(props, !props.options.alertsOnTop)}
onClick={(_e) => setAlertsOnTop(props, !props.options.alertsOnTop)}
lizzthabet marked this conversation as resolved.
Show resolved Hide resolved
analyticsName="ui.web.alertsOnTopToggle"
>
Alerts on Top
Expand Down
24 changes: 4 additions & 20 deletions web/src/ResourceStatusSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -285,37 +285,21 @@ type ResourceStatusSummaryProps = {
export function ResourceStatusSummary(props: ResourceStatusSummaryProps) {
// Count and calculate the combined statuses.
let resources = props.view.uiResources || []
const allStatuses: ResourceStatus[] = []

const testStatuses: ResourceStatus[] = []
const otherStatuses: ResourceStatus[] = []
resources.forEach((r) => {
const status = combinedStatus(buildStatus(r), runtimeStatus(r))
allStatuses.push(status)
if (r.status?.localResourceInfo?.isTest) {
testStatuses.push(status)
} else {
otherStatuses.push(status)
}
})
const allStatuses = resources.map((r) =>
combinedStatus(buildStatus(r), runtimeStatus(r))
)

return (
<ResourceStatusSummaryRoot>
<ResourceMetadata counts={statusCounts(allStatuses)} />
<ResourceGroupStatus
counts={statusCounts(otherStatuses)}
counts={statusCounts(allStatuses)}
label={"Resources"}
healthyLabel={"healthy"}
unhealthyLabel={"err"}
warningLabel={"warn"}
/>
<ResourceGroupStatus
counts={statusCounts(testStatuses)}
label={"Tests"}
healthyLabel={"pass"}
unhealthyLabel={"fail"}
warningLabel={"warn"}
/>
</ResourceStatusSummaryRoot>
)
}
Expand Down
31 changes: 1 addition & 30 deletions web/src/SidebarResources.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { accessorsForTesting, tiltfileKeyContext } from "./LocalStorage"
import {
AlertsOnTopToggle,
ResourceNameFilterTextField,
TestsHiddenToggle,
TestsOnlyToggle,
} from "./OverviewSidebarOptions"
import { assertSidebarItemsAndOptions } from "./OverviewSidebarOptions.test"
import PathBuilder from "./PathBuilder"
Expand Down Expand Up @@ -117,15 +115,11 @@ describe("SidebarResources", () => {
})

const falseyOptions: SidebarOptions = {
testsHidden: false,
testsOnly: false,
alertsOnTop: false,
resourceNameFilter: "",
}

const loadCases: [string, any, string[]][] = [
["tests only", { ...falseyOptions, testsOnly: true }, ["a", "b"]],
["tests hidden", { ...falseyOptions, testsHidden: true }, ["vigoda"]],
[
"alertsOnTop",
{ ...falseyOptions, alertsOnTop: true },
Expand Down Expand Up @@ -166,19 +160,11 @@ describe("SidebarResources", () => {
</MemoryRouter>
)

assertSidebarItemsAndOptions(
root,
expectedItems,
options.testsHidden,
options.testsOnly,
options.alertsOnTop
)
assertSidebarItemsAndOptions(root, expectedItems, options.alertsOnTop)
}
)

const saveCases: [string, SidebarOptions][] = [
["testsHidden", { ...falseyOptions, testsHidden: true }],
["testsOnly", { ...falseyOptions, testsOnly: true }],
["alertsOnTop", { ...falseyOptions, alertsOnTop: true }],
["resourceNameFilter", { ...falseyOptions, resourceNameFilter: "foo" }],
]
Expand All @@ -204,21 +190,6 @@ describe("SidebarResources", () => {
</MemoryRouter>
)

let testsHiddenControl = root.find(TestsHiddenToggle)
if (
testsHiddenControl.hasClass("is-enabled") !==
expectedOptions.testsHidden
) {
testsHiddenControl.simulate("click")
}

let testsOnlyControl = root.find(TestsOnlyToggle)
if (
testsOnlyControl.hasClass("is-enabled") !== expectedOptions.testsOnly
) {
testsOnlyControl.simulate("click")
}

let aotToggle = root.find(AlertsOnTopToggle)
if (aotToggle.hasClass("is-enabled") !== expectedOptions.alertsOnTop) {
aotToggle.simulate("click")
Expand Down