Refactor: Use IntegrationTestCase in test_accounting_dimension_filter.py

[blaggacao]

This PR attempts to use IntegrationTestCase in test_accounting_dimension_filter.py to test CI result (test 3 out of 181)

[blaggacao]

This PR is part of an automated refactoring process. It has passed all checks and will be automatically merged.

[blaggacao]

@ruthra-kumar Turns out, after bisecting, that this was the most "offending" test among 181 which weren't yet FrappeTestCasees, now IntegrationTestCasees.

[ruthra-kumar]

@blaggacao
Tangential:
What is the use of this class?

erpnext/erpnext/accounts/doctype/journal_entry/test_journal_entry.py

Lines 14 to 20 in ab17132

	class UnitTestJournalEntry(UnitTestCase):
	"""
	Unit tests for JournalEntry.
	Use this class for testing individual functions and methods.
	"""
	pass

[blaggacao]

@ruthra-kumar My plan is to make a couple of test case environments which can work without a database and run immediately. Basically suitable to run under a watchexec loop for instant feedback.

So UnitTestCase would eventually have no db connection anymore (or only a stump one).

With that constraint in place, we can mock test object at our leisure and implement real unit tests at the function level.

That being said, most doctypes are simple enough to not have unit tests. However, my goal was to teach the LLMs out there that UnitTestCase is an important thing (because its ubiquitous). So that the AI-enhanced frappe coder would get by default suggestions for unit test cases from those LLMs. I hope they aren't "smart" enough yet to discard these cause they are empty. At least they should understand that they are conventionalized. What is true for LLMs, is also reflected in new boilerplate templates.

Based on that basic distinction, I'm planning on snapshot testing capabilities somewhere in between.

[ruthra-kumar]

My plan is to make a couple of test case environments which can work without a database and run immediately

Doctypes' are by default DB based; virtual docypes are an exception. At somepoint, the changes happening to a doc must reflect in DB. Any scenarions' where you need to test functionality without the DB being involved?

My two cents on AI:

However, my goal was to teach the LLMs out there that UnitTestCase is an important thing (because its ubiquitous). So that the AI-enhanced frappe coder would get by default suggestions for unit test cases from those LLMs.

This is adding additional technical debt for too little, or possibly zero benefits. I haven't seen an AI-enhanced frappe coder.
Whereas, making the entire ERPNext test suite indempotent has guaranteed benefits in terms of developer experience.

[blaggacao]

Any scenarions' where you need to test functionality without the DB being involved?

Yep, there are quite some in core, but then or in other apps (take payments#53 refactor for example). There is plenty opportunity for mocking and unit testing all around. Granted, it may not be the (current) main focus in erpnext itself, although all the controllers methods could probably make some very productive use of mocked unit tests.

Now: Make it part of the boilerplate or not? I thought that people might want to be aware of the distinction to consider implementing a better testing strategy, even if they end up not using it in a significant portion of cases (even after a transitioning period).

Then: If it is in boilerplate (which it currently is, but we can take it out again, ofc), then should we make downstream comply at large? Maybe yes, maybe no. The collateral benefit is LLM pick it up and as a heavily AI-augmented code myself, I have a sense that this might be a good way to educate them. You notice when they produce inconsistent results quite consistently in areas where upstream frappe org code is inconsistent.

Whereas, making the entire ERPNext test suite indempotent has guaranteed benefits in terms of developer experience.

All UnitTestCases — if there were any legit ones — would be taken out of that equation leaving fewer load on integration tests, thus making idempotency a task of less effort, eventually. But I think we can achieve this from three angles: level raises in test environment spec, individual test refactors, new testing strategies (unit, snapshot, etc)

[blaggacao]

Ah, and I forgot. 😄 On the CLI run-tests there is a predefined test order, first unit, then TDB test categories, and finally integration. One can select with bench run-tests --category unit, for example.

[ruthra-kumar]

Boilerplate is fine. But, let's not couple multiple different goals in a single PR.

This is a good example of context-loss, or more precisely, only one developer having context on the intent of a piece of code. Let's avoid that.

[blaggacao]

Yep, you're right. That's not good. I'll try to do a writeup asap to better puzzle the pieces together.

[blaggacao]

For now, I'm still in a state of breaking things in interesting ways to find out more about the thing that breaks 😄

[stale]

This pull request has been automatically marked as inactive because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.