Fix: Fixed the broken integration tests after the removal of old Mongo - #152
Conversation
d5b405a to
077374f
Compare
arslanashraf7
left a comment
arslanashraf7
left a comment
There was a problem hiding this comment.
I tried multiple solutions to fix the tests instead of creating ScopeIds directly and passing a hard coded definition_id. But they might require more debugging. At the very least I left a couple of comments that might help you do the suggested changes or debug this a little further.
| xblock.fields.ScopeIds: A ScopeIds object for the block for usage_key | ||
| """ | ||
| block_type = 'fake' | ||
| def_id = runtime.id_generator.create_definition(block_type) |
There was a problem hiding this comment.
Can we use TestRutime from edX here? That's specifically created for Testing Purposes.
Ideally, It would look like :
from xblock.runtime import DictKeyValueStore, KvsFieldData
from xblock.test.tools import TestRuntime
runtime = TestRuntime(services={'field-data': KvsFieldData(kvs=DictKeyValueStore())})
The imports can go on top of the file thought. I was testing different things out and the tests still pass if we use id-generator with the above runtime.
| @@ -112,8 +109,9 @@ def import_test_course(self): | |||
| store = modulestore() | |||
There was a problem hiding this comment.
From my above comment, I also tried using TestRuntime here as part of preparing runtime for the user. I was still able to get most of the tests to pass except for the 12 tests that were failing. You can try this as well if you want. If you go with this approach, you can create a TestRuntime in prepare runtime method and passing that to the runtime instead of self.block.runtime.
In this case: 12 tests that are failing are related to the fragment not being loaded properly in the student/studio view.
There was a problem hiding this comment.
The test cases which are passing do not rely on the runtime. The cases that are failing are the ones using the runtime. I also explored TestRuntime and discovered that it's simply a dummy class for getting the tests to run. In our case, we require a proper runtime to execute some tests.
There was a problem hiding this comment.
I agree. TestRuntime provides the basic runtime testing structure. This could however evolve over time whereas our hard-coded definition_id won't be. So, It would be better to use the basic skeleton of TestRuntime to keep things dynamically moving over the future.
arslanashraf7
left a comment
arslanashraf7
left a comment
There was a problem hiding this comment.
Looks good. 👍 Just one minor change and you can merge this.
Note: If needed, Rebase your PR before merge and do a squash merge.
| from xblock.runtime import DictKeyValueStore, KvsFieldData | ||
| from xblock.test.tools import TestRuntime |
There was a problem hiding this comment.
These imports should be grouped with xBlock imports above on line#10
dff30e5 to
57014f1
Compare
2 participants
What are the relevant tickets?
#142
What's this PR do?
This PR fixes the integration tests which were broken after the removal of old Mongo support for parent child relationship
Our tests started to break after the openedx/openedx-platform#31134 was merged giving us an ScopeError
This PR changes the tests to use the new SplitMongo modulestore
See more information in the relevant issue ticket
How should this be manually tested?
srcfolder of your devstack work directory./run_devstack_integration_tests.sh