Prefer test units in UI tests#66876
Conversation
e7cff57 to
75f187b
Compare
0f00c3e to
1e7694d
Compare
29c3b73 to
903e7b5
Compare
11562c0 to
44bdced
Compare
|
CC @bethanyaconnor since this touches course2 which you were working on deprecating |
| | http://studio.code.org/courses/20-hour/units/1/lessons/2/levels/1?noautoplay=true | first maze level | | ||
| | http://studio.code.org/courses/course2/units/1/lessons/7/levels/2?noautoplay=true | artist level | |
There was a problem hiding this comment.
I'm not sure how these were still working 😬 thanks for updating!
There was a problem hiding this comment.
ahh ok, that explains it. I see just one other reference to course1-4, which is also skipped:
It seems like it should be safe to stop seeding course1 thru course4 in test entirely, then. can you remind me what other courses I should maybe be checking for? it seems like 20-hour still has some references in other UI tests, and I don't recall what other courses got deprecated if any.
There was a problem hiding this comment.
It's just courses 1-4 and 20-hour. One of my next cleanups was to remove them from seeding, though when I tried to do that before I even deprecated the courses, Drone still passed, so I'm not sure how to gain confidence...
There was a problem hiding this comment.
I can help you with that! It has to do with the fact that .drone.yml is configured to generate a cached build (including seeded content) from the most recent staging branch, and then use that as the basis for each drone run. I'm glad you mentioned it because I should really be running it against this PR. So that I don't have to jump through hoops twice, I can just take care of removing course1-4 from the seed steps in this PR.
I'm confused about 20-hour though. are you just still in the process of removing that course from our UI tests?
There was a problem hiding this comment.
20-hour should be deprecated, but I mostly looked at what failed when I removed it and I'm realizing tests like dashboard/test/ui/features/student_learning/hour_of_code/hour_of_code.feature actually just check that the progress in the header is there, which is still is even with the deprecation screen 🙃
There was a problem hiding this comment.
I see. IIRC you replaced 20-hour with Unit.hoc_2014_unit as the default unit in a bunch of places, possibly including the progress view. if we want to restore the usefulness of those tests, the current references could be updated to point to the new default unit instead. however, I'm ostensibly going to go through and try to eliminate all references to user-facing units, so I think it's probably best to just punt on that for now.
There was a problem hiding this comment.
the seeding-without-cache check is complete, ready for another look.
|
Looks like artist is still needed here: 
I could change the test to point to a different unit, but since the only visible units are user-facing, that won't help me in the long run, so I'm just going to keep seeding artist for now. |
|
^ just noting that I went ahead and made the updates to allthethings via levelbuilder, to reduce risk of merge conflicts on allthethings while this PR is in review. |
alex-m-brown
left a comment
There was a problem hiding this comment.
This looks like some great steps towards cleaning this up!
bethanyaconnor
left a comment
There was a problem hiding this comment.
🎉 thanks for doing this cleanup!
This PR updates several UI tests to use levels in the allthethings course rather than levels used by our end users, with the end goal of eliminating our dependency on user-facing curriculum in UI tests.
The following levels were added to the end of the specified lessons in the allthethings unit:
Then individual UI tests were updated to refer to the newly added levels.
Testing story
Since this change affects only test code and curriculum, I am relying on the passing drone run on existing tests for test coverage.