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

feat: render pipeline warnings as line annotations #831

Merged
merged 9 commits into from
Jan 10, 2025
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions cypress/fixtures/pipeline_warnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"id": 1,
"repo_id": 1,
"commit": "9b1d8bded6e992ab660eaee527c5e3232d0a2441",
"flavor": "",
"platform": "",
"ref": "refs/heads/main",
"type": "yaml",
"version": "1",
"external_secrets": false,
"internal_secrets": false,
"services": false,
"stages": false,
"steps": true,
"templates": true,
"warnings": ["pipeline is using shared secrets", "4:invalid template name"],
"data": "dmVyc2lvbjogIjEiCnN0ZXBzOgotIHRlbXBsYXRlOgogICAgbmFtZTogY3VzdG9tX3RlbXBsYXRlCiAgICB2YXJzOgogICAgICBidWlsZF9ldmVudDogJHtCVUlMRF9FVkVOVH0KICBuYW1lOiBpbnZva2UgdGVtcGxhdGUxX25hbWUKICBwdWxsOiBub3RfcHJlc2VudAotIHRlbXBsYXRlOgogICAgbmFtZTogc2ltcGxlX3RlbXBsYXRlCiAgICB2YXJzOgogICAgICBidWlsZF9ldmVudDogJHtCVUlMRF9FVkVOVH0KICBuYW1lOiBpbnZva2UgdGVtcGxhdGUyX25hbWUKICBwdWxsOiBub3RfcHJlc2VudAotIHRlbXBsYXRlOgogICAgbmFtZTogYnJhbmNoZWRfdGVtcGxhdGUKICAgIHZhcnM6CiAgICAgIGJ1aWxkX2V2ZW50OiAke0JVSUxEX0VWRU5UfQogIG5hbWU6IGludm9rZSB0ZW1wbGF0ZTNfbmFtZQogIHB1bGw6IG5vdF9wcmVzZW50CnRlbXBsYXRlczoKLSBuYW1lOiB0ZW1wbGF0ZTFfbmFtZQogIHNvdXJjZTogZ2l0aHViLmNvbS9naXRodWIvb2N0b2NhdC90ZW1wbGF0ZTEueW1sCiAgdHlwZTogZ2l0aHViCi0gbmFtZTogdGVtcGxhdGUyX25hbWUKICBzb3VyY2U6IGdpdGh1Yi5jb20vZ2l0aHViL29jdG9jYXQvdGVtcGxhdGUyLnltbAogIHR5cGU6IGdpdGh1YgotIG5hbWU6IHRlbXBsYXRlM19uYW1lCiAgc291cmNlOiBnaXRodWIuY29tL2dpdGh1Yi9vY3RvY2F0L3RlbXBsYXRlMy55bWwKICB0eXBlOiBnaXRodWI="
}
125 changes: 125 additions & 0 deletions cypress/integration/pipeline.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ context('Pipeline', () => {
cy.get('[data-test=pipeline-expand]').should('exist');
});

it('warnings should not be visible', () => {
cy.get('[data-test=pipeline-warnings]').should('not.be.visible');
});

context('click expand templates', () => {
beforeEach(() => {
cy.get('[data-test=pipeline-expand-toggle]').click({
Expand Down Expand Up @@ -285,4 +289,125 @@ context('Pipeline', () => {
});
},
);
context(
'logged in and server returning valid pipeline configuration (with warnings) and templates',
() => {
beforeEach(() => {
cy.server();
cy.stubBuild();
cy.stubPipelineWithWarnings();
cy.stubPipelineExpand();
cy.stubPipelineTemplates();
cy.login('/github/octocat/1/pipeline');
});

it('warnings should be visible', () => {
cy.get('[data-test=pipeline-warnings]').should('be.visible');
});

it('should show 2 warnings', () => {
cy.get('[data-test=pipeline-warnings]')
.children()
.should('have.length', 2);
});

it('warning with line number should show line number button', () => {
cy.get('[data-test=warning-line-num-4]')
.should('be.visible')
.should('not.have.class', '-disabled');
});

it('warning with line number should show content without line number', () => {
cy.get('[data-test=warning-0] .line-content')
.should('be.visible')
.should('not.contain', '4')
.should('contain', 'template');
});

it('warning without line number should replace button with dash', () => {
cy.get('[data-test=warning-1]')
.should('be.visible')
.should('contain', '-');
});

it('warning without line number should content', () => {
cy.get('[data-test=warning-1] .line-content')
.should('be.visible')
.should('contain', 'secrets');
});

it('log line with warning should show annotation', () => {
cy.get('[data-test=warning-annotation-line-4]').should('be.visible');
});

it('other lines should not show annotations', () => {
cy.get('[data-test=warning-annotation-line-5]').should(
'not.be.visible',
);
});

context('click warning line number', () => {
beforeEach(() => {
cy.get('[data-test=warning-line-num-4]').click({ force: true });
});

it('should update path with line num', () => {
cy.hash().should('eq', '#4');
});

it('should set focus style on single line', () => {
cy.get('[data-test=config-line-4]').should('have.class', '-focus');
});

it('other lines should not have focus style', () => {
cy.get('[data-test=config-line-3]').should(
'not.have.class',
'-focus',
);
});
});

context('click expand templates', () => {
beforeEach(() => {
cy.get('[data-test=pipeline-expand-toggle]').click({
force: true,
});
cy.wait('@expand');
});

it('should update path with expand query', () => {
cy.location().should(loc => {
expect(loc.search).to.eq('?expand=true');
});
});

it('should show pipeline expansion note', () => {
cy.get('[data-test=pipeline-warnings-expand-note]').contains('note');
});

it('warning with line number should show disabled line number button', () => {
cy.get('[data-test=warning-line-num-4]')
.should('be.visible')
.should('have.class', '-disabled');
});

context('click warning line number', () => {
beforeEach(() => {
cy.get('[data-test=warning-line-num-4]').click({ force: true });
});

it('should not update path with line num', () => {
cy.hash().should('not.eq', '#4');
});

it('other lines should not have focus style', () => {
cy.get('[data-test=config-line-3]').should(
'not.have.class',
'-focus',
);
});
});
});
},
);
});
10 changes: 10 additions & 0 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,16 @@ Cypress.Commands.add('stubPipeline', () => {
});
});

Cypress.Commands.add('stubPipelineWithWarnings', () => {
cy.fixture('pipeline_warnings.json').as('pipeline');
cy.route({
method: 'GET',
url: '*api/v1/pipelines/*/*/*',
status: 200,
response: '@pipeline',
});
});

Cypress.Commands.add('stubPipelineErrors', () => {
cy.route({
method: 'GET',
Expand Down
2 changes: 1 addition & 1 deletion src/elm/Components/Logs.elm
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ viewLine pushUrlHashMsg resourceType resourceNumber shift focus logLine lineNumb
]
[ span [] [ text <| String.fromInt lineNumber ] ]
]
, td [ class "break-text", class "overflow-auto" ]
, td [ class "break-text", class "overflow-auto", class "line-content" ]
[ code [ Util.testAttribute <| String.join "-" [ "log", "data", resourceNumber, String.fromInt lineNumber ] ]
[ logLine.view
]
Expand Down
17 changes: 16 additions & 1 deletion src/elm/Components/Svgs.elm
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ SPDX-License-Identifier: Apache-2.0


module Components.Svgs exposing
( buildStatusAnimation
( annotationCircle
, buildStatusAnimation
, buildStatusToIcon
, buildVizLegendEdge
, buildVizLegendNode
Expand Down Expand Up @@ -792,3 +793,17 @@ buildVizLegendEdge attrs =
)
[]
]


{-| annotationCircle : produces svg icon for line annotation bubble.
-}
annotationCircle : String -> Html msg
annotationCircle cls =
svg
[ class "-icon"
, class cls
, viewBox "0 0 48 48"
, width "22"
, height "22"
]
[ Svg.circle [ cx "24", cy "24", r "8" ] [] ]
Loading
Loading