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

chore: add test to verify create fuels template integrity #2364

Merged

Conversation

petertonysmith94
Copy link
Contributor

@petertonysmith94 petertonysmith94 commented May 21, 2024

Closes #2265

@petertonysmith94 petertonysmith94 added the chore Issue is a chore label May 21, 2024
@petertonysmith94 petertonysmith94 self-assigned this May 21, 2024
@petertonysmith94 petertonysmith94 changed the title Ps/chore/add automated test suite for create fuels chore: add automated test suite for create fuels May 21, 2024
Copy link
Contributor

github-actions bot commented May 21, 2024

This PR is published in NPM with version 0.0.0-pr-2364-20240522151632

@petertonysmith94 petertonysmith94 changed the title chore: add integration test for create fuels CLI chore: add e2e test for create fuels CLI May 23, 2024
Copy link
Member

@maschad maschad left a comment

Choose a reason for hiding this comment

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

A few comments:

  • I think the title of this PR should be chore: add test to verify create-fuels template

  • given that this PR doesn't necessarily add any functionality that we would use without chore: add integration test for the create fuels template #2278 I think it should be based off the dp/create-fuels-template-e2e-tests branch

  • When running this locally the tests don't seem to pass for me.

Screenshot 2024-05-23 at 11 52 05 AM

@petertonysmith94 petertonysmith94 changed the title chore: add e2e test for create fuels CLI chore: add test to verify create fuels template integrity May 23, 2024
@petertonysmith94
Copy link
Contributor Author

A few comments:

  • I think the title of this PR should be chore: add test to verify create-fuels template
  • given that this PR doesn't necessarily add any functionality that we would use without chore: add e2e test for the create fuels template #2278 I think it should be based off the dp/create-fuels-template-e2e-tests branch
  • When running this locally the tests don't seem to pass for me.
Screenshot 2024-05-23 at 11 52 05 AM

@maschad
Copy link
Member

maschad commented May 23, 2024

As part of the overarching #2170: we will perform the merger of this PR and #2278 to get the full test. We believe they both serve value on their own, and can go out independently of each other as an increment of work which delivers value.

Maybe I'm missing something but I am not sure what the use case for this test is outside of #2278

@petertonysmith94
Copy link
Contributor Author

petertonysmith94 commented May 24, 2024

Maybe I'm missing something but I am not sure what the use case for this test is outside of #2278

#2278 is adding UI testing for our template, against a local node.

#2364 is adding tests around performing create fuels using a publish version from NPM and verifying the templates we ship are correct and have all the correct files.

#2170 will be adding a test that performs create fuels from a published version from NPM and performing the UI testing.

I think it would be more productive to have a call with @Dhaiwat10 and myself over this is you are still unsure.

@nedsalk nedsalk self-requested a review May 24, 2024 09:48
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
46.37%(+0%) 39.55%(+0%) 43.6%(+0%) 46.2%(+0%)
Changed Files:

Coverage values did not change👌.

@petertonysmith94 petertonysmith94 merged commit 3b00bdb into master May 24, 2024
19 checks passed
@petertonysmith94 petertonysmith94 deleted the ps/chore/add-automated-test-suite-for-create-fuels branch May 24, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issue is a chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a 'live' automated testing suite for create fuels
6 participants