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.

Sign up for GitHub

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

Jump to bottom

Acceptance tests for top-level functions #589

Open
jmfayard opened this issue Sep 7, 2022 · 1 comment · May be fixed by #644
Open

Acceptance tests for top-level functions #589

jmfayard opened this issue Sep 7, 2022 · 1 comment · May be fixed by #644
Labels
ready-for-development Grab it while you can! refactoring beautiful code

Comments

@jmfayard
Copy link
Member

jmfayard commented Sep 7, 2022

This is my current understanding of the problem. Please provide feedback whether I'm missing something.

We have lots of top-level functions and it's often unclear

  • issue1: whether they belong to the functional core (not the imperative shell) and therefore needs to be tested
  • issue2: whether they are currently being tested
  • issue3: if not, what kind of simple tests we can add as a starting point

For example with the function getArtifactNameToConstantMapping()

  • issue1: definitely part of the functional core and we relay on it for multiple features
  • issue2: it's not being tested currently. It's used in other tests which is not the same thing.
  • issue3: the function works so we should not modify it but simply write what Martin Fowler calls acceptance tests for it. Meaning: if we give her this kind of objects as input, this is the output we currently get.

Regarding issue 2, an issue is that if you have a class FindKitten, there is a strong convention that you test it in the class FikkndKittenTest. And you can jump there with the action Go To Test.

But there is no such convention for top-level functions AFAIK.
@jmfayard
Copy link
Member Author

jmfayard commented Sep 7, 2022

Do a code review and find the functions without side effects that are part of the functional core.
If they are tested, add an annotation @TestedBy("SomethingTest")
If they are not, add an annotation @NeedsTest

Add acceptance tests for important functions like getArtifactNameToConstantMapping()

Alternative: if there is a bunch of top-level functions that belong together, put them inside an object or a class (if they have multiple common parameters) so that we use the convention FindKitten -> FindKittenTest

@jmfayard jmfayard added refactoring beautiful code needs-feedback not enough feedback to start working on this and removed not-for-this-release labels Sep 13, 2022
@jmfayard jmfayard added ready-for-development Grab it while you can! and removed needs-feedback not enough feedback to start working on this labels Oct 30, 2022
@jmfayard jmfayard changed the title Top-level functions and testing Acceptance tests for top-level functions Oct 30, 2022
@jmfayard jmfayard linked a pull request Oct 31, 2022 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ready-for-development Grab it while you can! refactoring beautiful code
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Annotations @FunctionalCore and @NeedsRefactoring Splitties/refreshVersions
1 participant
@jmfayard