Skip to content

Generate project test files#27

Open
Frozenlock wants to merge 9 commits intoachin:masterfrom
Frozenlock:generate-project-test-files
Open

Generate project test files#27
Frozenlock wants to merge 9 commits intoachin:masterfrom
Frozenlock:generate-project-test-files

Conversation

@Frozenlock
Copy link

(based on PR #26)

This is the major refactoring I previously hinted at.
I don't have any other change planned, so at this point I'll wait for
the PRs to be merged and a potential new release.

Previously the various test 'project.clj' files were manually
created. It quickly became confusing as the number of tests
increased.

The new approach keeps all the configurations under test at the same
place, avoiding any spillover from one test to another and making it
easier to read and maintain.

@achin
Copy link
Owner

achin commented Jul 5, 2020

Thanks again for your pull requests. I'm going through them and trying to understand the changes that you made.

I always support improving test code, especially increasing clarity. I realize that manually creating the projects-under-test and checking them into source control is manual and repetitive, but I feel that its straightforwardness makes understanding the tests easier. Introducing a framework that generates and manages these templates creates an additional layer to understand and debug, which has an impact on overall clarity.

What do you think?

@Frozenlock
Copy link
Author

My point wasn't about being manual and repetitive, but being confusing and prone to spillovers from one test to another as the project files are re-used.

For example, let's say that in a test I decide to test :property2 instead of :property1. Can I remove property1 form the project file? Not without checking all other tests and making sure none of them is relying on this property. In fact, even adding a property could break a test, for example testing a default value. It wouldn't be so bad if breaking tests would make them fail, but it's possible to break them in a way to that will succeed without ever testing what they were written for.

Also, the naming scheme doesn't really scale with the number of tests; the names need to be ever more precise for small difference in properties.

An alternative would be to never use a project file more than once.
Thus test1 would have a dedicated directory containing both the parent and child:
/test1/parent/project.clj
/test1/child/project.clj

@achin
Copy link
Owner

achin commented Jul 5, 2020

That makes a lot of sense, and I see where you're coming from. My only remaining apprehension is irrational: I'm not familiar with and don't completely understand the new framework. I don't feel like I can really review it properly because I don't know how it interacts with everything else or the other changes.

I can merge your PRs into a branch and we can go from there. It looks like this PR is based off of PR #26. Have I understood correctly that I should merge PRs #27 (this one), #25, and #24? And abandon #26?

@Frozenlock
Copy link
Author

Frozenlock commented Jul 5, 2020

Those were built on top of each other:
#24 -> #26 -> #27
(So #27 contains everything)

Not sure if they should be merged one after the other.

IIRC #25 is standalone and can be merged whenever.

@Frozenlock
Copy link
Author

Hello there!

Any update on this?

@vemv
Copy link
Contributor

vemv commented Apr 21, 2022

Hi,

at the company I work for we'd be interested in all four PRs, they seem good stuff. We'd need #26 particularly.

Would much appreciate salvaging these!

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.

3 participants