Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: step 2 refactor towards idempotency #43612

Merged

Conversation

blaggacao
Copy link
Collaborator

@blaggacao blaggacao commented Oct 11, 2024

  • refactor: use cls.globalTestRecords instead of declaring test_records
    As with test: step 1 refactor towards idempotency #43610 test_records may be considered an anti-pattern if it doesn't deviate from the default behaviour. Instead, we ensure proper dependency declaration, and use self.globalTestRecords (actually: cls.globalTestRecords)

cc @ruthra-kumar @rohitwaghchaure @s-aga-r @deepeshgarg007

If we can get this to green, then this is a significant step, because it means that we have achived (or maybe only ensured, cause it already was the case) a baseline for a certain locality and isolation of tests: items inside cls.globalTestRecords are MappingProxys and therefore can't be written to. They represent the records at their time of insertion into the database. On a rerun on the CLI with the same database, they are reconstructed from sites/test_site/.test_log.jsonl instead of from an actual insert and represent the records at the time of their original insert on prior test runs.

This PR also significantly reduces the deprecation warning pressure on CLI.


Intermediate Step Required

AttributeError: 'mappingproxy' object has no attribute 'as_dict'

This is wonderful! It shows that the mapping proxies are working as intended. On the framework side, we want to modify at least frappe.copy_doc to work with MappingProxy, as of its widespread use in tests.


enterClassContext / enterContext technique

All frappe.tests.classes.context_managers can be used in setUp or setUpClass with enterContext or enterClassContext. This is a relatively new unittest feature which takes a context manager and ensures the context is upheld during the entire scope of the test class / test case respecitvely.

An example of its use: c17de26


Further Intemediate Steps Required

These mainly fixed the Test Record loading for correctness. The most significant change was to load actual json records into cls.globalTestRecords instead of doc.as_dict() post db-insertion. This matches more closely the expectations that test designers had when they previously declared test_records = ....

@blaggacao blaggacao force-pushed the refactor/alignment-in-test-record-creation branch 5 times, most recently from efd6647 to 8fa570d Compare October 11, 2024 06:20
@blaggacao blaggacao force-pushed the refactor/alignment-in-test-record-creation branch 4 times, most recently from 4b92d77 to db91b1f Compare October 12, 2024 11:17
@blaggacao blaggacao force-pushed the refactor/alignment-in-test-record-creation branch from db91b1f to 8867702 Compare October 12, 2024 22:07
@blaggacao blaggacao marked this pull request as ready for review October 12, 2024 22:28
@blaggacao blaggacao merged commit cc954d9 into frappe:develop Oct 12, 2024
15 checks passed
@blaggacao blaggacao deleted the refactor/alignment-in-test-record-creation branch October 12, 2024 22:31
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.

1 participant