PR 1733 included some changes to EditorPipelineGraph.jsx. These changes provided an id for automated tests, so that stages could be targeted by their stage name. Ivan Meredith correctly pointed out that IDs should not have spaces in them, and as things stand, they could. This ticket is being created to fix that up.
There are two ways we could do this.
- Add some string substitution to the ID, to remove spaces
- Switch instead to a data tag. Something along the lines of data-stage-name="stage name with spaces in it"
The data tag approach makes more sense to me. If, in the confines of the test, a stage is created with the name my stage name with spaces, we'd have the JS side doing something to convert the spaces to something else. But we'd also have to change subsequent test code, to look for the "corrected" stage name. In other words, you can't just pass around a string with the value my stage name with spaces, because the editor will have removed those spaces. It seems the test code will need to be aware of this and do a conversion of its own.
Here's the example from PR 1733, in the order the steps get executed:
- Initialize a variable with the name of our stage, with spaces in it
- Create the stage
- Later, we come back and delete the same stage, and we do so by its name. Meaning we're passing it with the spaces intact.
You can see what will happen above if we strip the spaces out of the stage name on the JS side. The test won't be able to find it and will fail. So it seems like using a data tag instead would make more sense. In this way, we could pass around a stage named whatever we like, without having to change things.
Of course, we could just not use stage names with spaces in them. But this is something a user is going to do, so we should test using real-world stage names. What's more, other people writing test cases might want to target stages for deletion by their names.