add acceptance tests for event tracking - #1207
Conversation
There was a problem hiding this comment.
You should include an error message with this assertion containing which expected field wasn't found. Nose assertions allow a msg kwarg; assert_in might be a good one here.
There was a problem hiding this comment.
Didn't know nose exposed all of the TestCase methods for me, that's awesome! Fixed.
|
I'm really happy to see an LMS/Analytics integration test, and I agree with this approach. I think the build failure might have been a casualty of GitHub going down this afternoon, so you should re-trigger the job manually. |
There was a problem hiding this comment.
Shouldn't we tested against the http.request event instead? I am amusing the in the last line means event_type.
There was a problem hiding this comment.
I'm putting this patch in before the other one, so it is simply verifying the current event format, where event_type = url for page view events.
Once I integrate with edx platform I'd like to continue to verify the old events, but also include so kind of verification for new events as well.
|
The video.feature test fails on Jenkins but not on my local environment... I suspect some kind of race condition. I'm currently trying to figure out the best way to debug the issue. @wedaly - do we have direct ssh access to the jenkins slave? Also, is there some place I can "see" the configuration of the Jenkins job? I want to make sure I'm running the same command locally as Jenkins is running... |
|
@mulby I think there's a non-deterministic test in video.feature -- when I ran master a bunch of times this weekend, it failed intermittently. I'm working on getting that fixed now. That's one of the benefits of running the tests more frequently; there's an incentive to get things fixed :) We can directly ssh into the jenkins slave, but it involves some setup to get into the VPC. The Jenkins job delegates to the rake tasks to run the tests: where One of my goals for the hackathon is to set up vagrant to exactly duplicate the Jenkins worker configuration, which should make it easier to track and fix these kinds of failures. |
|
@wedaly - I removed the video.feature test and replaced it with a couple of problem related event tests. Hopefully that's fine for now. Pretty soon we are going to start "touching" most of the emitted events, so I'm planning on adding coverage for each event as we go. @rocha, @wedaly - after squashing is this good to merge (assuming tests pass)? |
|
@rocha - are we supposed to be tracking events in the CMS? |
|
👍 |
|
LGTM @mulby yes, we are capturing some events on the CMS, mostly on the server side. I feel it is not comprehensive. Also, looks like you need to rebase 😕 |
|
@rocha - yeah I'm going to rebase and squash before merging this. I'm going to defer adding acceptance tests to the CMS for now. I'm not really clear on how that data is going to be used. |
add acceptance tests for event tracking
…ew_problem_for_dogwood_release Fix few problem for dogwood release openedx#1167
By using the correct health check (TCP, rather than UNIX socket) we can get rid of the 10 second sleep. See docker-library/mysql#930 for discussion of health checks. This commit also simplifies the wait-for-mysql code in the provisioning script.
3 participants
I'm not sure if this is an appropriate direction, might be too nuts-and-bolts for an acceptance test. However, if you think of our researchers as users (of the data, not the site), then I think it makes more sense.
I'm not thrilled with the usage mongo here, I toyed around with creating an "in memory backend" that would just store the events in a list or something, but figured it was better to get coverage of the entire pipeline.
Reviewers: @rocha, @wedaly