Skip to content

Create a data_context.pickle and add a simple test for the data context stored in that pickle file#49

Merged
aaronayres35 merged 4 commits into
masterfrom
test/add-pickle-and-long-term-test
Mar 19, 2021
Merged

Create a data_context.pickle and add a simple test for the data context stored in that pickle file#49
aaronayres35 merged 4 commits into
masterfrom
test/add-pickle-and-long-term-test

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

Follow up of #48 see comment: #48 (review)

This PR adds a function to create a pickled data context file in tests/data/data_context.pickle which is only intended to be run once (I ran it right now and the generated file is included in the PR). Maybe I should throw out the function itself, to avoid it being run and overriding the current pickle? I left it so it is clear what the stored data context should look like.

It also adds a test that mirrors the existing test_persistence only using the long stored file. I am not sure if we want to run more tests on the store file? It seems if this tests passes, the other would be redundant so for now I have only included the simple one to check we can appropriately load the DataContext and get the object we would expect.

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost LGTM. CI might be unhappy because you're not including the .pickle file in the package. You'll need to update package data or the manifest file.

Comment thread codetools/contexts/tests/data_context_test_case.py Outdated
Comment thread codetools/contexts/tests/data_context_test_case.py Outdated
aaronayres35 and others added 2 commits March 19, 2021 08:23
Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
@aaronayres35 aaronayres35 merged commit 545bf68 into master Mar 19, 2021
@aaronayres35 aaronayres35 deleted the test/add-pickle-and-long-term-test branch March 19, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants