-
Notifications
You must be signed in to change notification settings - Fork 6
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
Shape Up #1 - e2e tests #2099
Shape Up #1 - e2e tests #2099
Conversation
You can access the deployment of this PR at https://renku-ci-ui-2099.dev.renku.ch |
7e32a36
to
5ffaa37
Compare
880850e
to
dbd5403
Compare
7988749
to
7b4808f
Compare
dbd5403
to
75981a5
Compare
75981a5
to
1d6faf4
Compare
f4cd85c
to
484f2c5
Compare
1d6faf4
to
afe0d63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for covering all those cases 🙌, I see almost everything covered, the only case missing would be filtering by disabled workflows. I see the code fine, I only found two small suggestions for changes.
@@ -574,7 +577,7 @@ function WorkflowDetailVisualizer({ | |||
return (<div className="workflows-detail-table"> | |||
<Row> | |||
<WorkflowVisualizerSimpleBox title="Inputs"> | |||
<VisualizerCommandEntities data={details.inputs} expanded={expanded} | |||
<VisualizerCommandEntities data-cy="AAA" data={details.inputs} expanded={expanded} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What means AAA? could we use a more intuitive variable here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that slipped in! It was a test I forgot to clean up 🤦 Thanks for spotting it!
getWorkflows(resultFile = "workflows/workflows-list-links-mappings.json") { | ||
cy.intercept( | ||
"/ui-server/api/renku/*/workflow_plans.list?*", | ||
{ fixture: resultFile } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please use this.useMockedData to indicate if the mocked file is used or not, so far we have not implemented the switch between mocked or not but we prefer to write the test including that variable to ease the transition.
const fixture = this.useMockedData ? { fixture: resultFile } : undefined;
Thank you for the review! I fixed the 2 points you pointed out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Tearing down the temporary RenkuLab deplyoment for this PR. |
re: https://github.com/SwissDataScienceCenter/renku-rfc/blob/e2d4a6ba02b6ec51914f35e83370ae15537e72a4/feature-pitches/001-project-workflows-ui-view/001-project-workflows-ui-view.md (#2041) (#2050) (#2065) (#2071) (#2082) (#2087) (#2092) (#2102) (#2105) (#2099) Co-authored-by: Andrea Cordoba <43388408+andre-code@users.noreply.github.com>
Covering the basic workflows functionalities with e2e tests
/deploy renku-core=develop renku=000-fix-acceptance-tests-after-changing-gitlab-url #persist extra-values=gateway.gitlabUrl=https://gitlab.dev.renku.ch/