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

Feature/dynamic tests #57

Merged
merged 20 commits into from
Dec 12, 2021
Merged

Feature/dynamic tests #57

merged 20 commits into from
Dec 12, 2021

Conversation

italojs
Copy link
Member

@italojs italojs commented Oct 5, 2021

Fixes #52

Proposed Changes

  • Generating unit test for any usecase present into generated project
  • Edited the doc
  • Rename folder from example/my-custom-entities to example-entities
  • removed new folder

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • Remember to check if code coverage decrease, if so, please implement the necessary tests

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@italojs italojs added the enhancement New feature or request label Oct 5, 2021
@italojs italojs marked this pull request as ready for review October 5, 2021 01:11
example-entities/profile/profile.js Outdated Show resolved Hide resolved
src/generators/src/domain/usecases/unitTests.js Outdated Show resolved Hide resolved
src/generators/src/domain/usecases/unitTests.js Outdated Show resolved Hide resolved
src/generators/src/domain/usecases/unitTests.js Outdated Show resolved Hide resolved
@jhomarolo
Copy link
Contributor

Hi @italojs could you resolve the conflicts here?

@italojs
Copy link
Member Author

italojs commented Oct 17, 2021

@dalssoft could you finish your review, please?

@dalssoft
Copy link
Member

bug: the tests files are created on herbs new but not on herbs update

│ ├── createUser.test.js
│ ├── deleteUser.js
│ ├── deleteUser.test.js
│ ├── deleteUser.js
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I don't think it make any sense to put the dir tree in the doc.

Copy link
Member Author

Choose a reason for hiding this comment

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

why not? I explains what is the final result that CLI generate, it's super explainable

/*{ <%= props.request %> }*/
const req = { nickname: 'herbsUser', password: 'V&eryStr0ngP@$$'}

const req = <%- props.request.valid %>
Copy link
Member

Choose a reason for hiding this comment

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

The generated value has multi lines and does not respect the spaces / tabs from the template.

Ex:

      }

      const req = {
    nickname: 'string',
    password: 'string'
}

      // When

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking to resolve it after resolve this issue #14, so the linter will fix it for us


it('should not delete the invalid <%= props.name.pascalCase %>', async () => {
// Given
const injection = {}
const user = aUser({ hasAccess: true })
/*{ <%= props.request %> }*/
Copy link
Member

Choose a reason for hiding this comment

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

bug:

      /*{ [object Object] }*/
      const req = { id : '5' }

Copy link
Member Author

Choose a reason for hiding this comment

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

sorry, could you explains a little bit more about the bug?

const useCases = Object.keys(validUseCaseRequests)

const valueType = {
String: "'string'",
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: a text, intead of string

const ret = await uc.run(req)

// Then
assert.ok(ret.isOk)
assert.strictEqual(ret.ok.nickname, 'herbsUser')
assert.strictEqual(ret.ok.isValid(), true)
Copy link
Member

Choose a reason for hiding this comment

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

assert.strictEqual(ret.ok.isValid(), true)
this assertion does not belong to this scenario (and all other tests with the same assertion) given that this test scenario test for UC running ok, not if the returned entity is valid

function aUser({ hasAccess }) {
return { hasAccess }
}
describe('Find one <%= props.name.raw %>', () => {
Copy link
Member

Choose a reason for hiding this comment

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

There should be a test to cover the scenario form if (!result) return Err.notFound({

Copy link
Member Author

Choose a reason for hiding this comment

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

done

function aUser({ hasAccess }) {
return { hasAccess }
}
describe('Update <%= props.name.raw %>', () => {
Copy link
Member

Choose a reason for hiding this comment

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

There should be a test for if (!ctx.user.isValid()) return Err.invalidEntity({

Copy link
Member Author

@italojs italojs Nov 21, 2021

Choose a reason for hiding this comment

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

It will need to be implemented in the future because the if (!ctx.user.isValid()) return Err.invalidEntity({ validation is applied for entities that has fields validation, this PR don't contemplate it.

@jhomarolo
Copy link
Contributor

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@italojs italojs deleted the feature/dynamic-tests branch May 4, 2022 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Herbs project must to born with unit tests for dynamic entites
3 participants