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

feat: added testing for new #212

Merged
merged 12 commits into from
Feb 28, 2022
Merged

Conversation

Samridhi-98
Copy link
Contributor

@Samridhi-98 Samridhi-98 commented Feb 2, 2022

Description

  • Added new.test.ts

Todo

  • Add startStudio.test.ts

Related issue(s)

Fixes #106

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@Samridhi-98
Copy link
Contributor Author

@fmvilas please review my PR 🙂

@fmvilas fmvilas requested a review from Souvikns February 2, 2022 19:24
@fmvilas
Copy link
Member

fmvilas commented Feb 2, 2022

Adding @Souvikns as I'm not a maintainer of this repo :)

@Souvikns
Copy link
Member

Souvikns commented Feb 3, 2022

@Samridhi-98 firstly thanks for opening PR we really appreciate it 👍🏻

The new command generates specification files. It is using inquirer to prompt for inputs. In the test environment, it is waiting for prompt input but not getting it. Due to this, you see the timeout exceeded error. Let me know if this makes sense.

Here you have to test for stdin and you can check out https://stackoverflow.com/questions/56797392/oclif-prompt-testing for some idea.

@Samridhi-98
Copy link
Contributor Author

@Souvikns I have implemented the changes but still getting a bunch of errors, Also I have doubts related to @oclif/test will you please provide me some resources. 🙂

@Samridhi-98 Samridhi-98 marked this pull request as draft February 3, 2022 18:38
@Souvikns Souvikns marked this pull request as ready for review February 4, 2022 10:42
Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

The issue you see is because of the prompt testing, I tried finding some examples with inquirer but wasn't able to find anything.

I tried running the test in uninteractive mode it works fine

describe('new', () => {
    test
        .stderr()
        .stdout()
        .do(() => {
            testHelper.createAsyncapiFile('specification.yaml', 'default-example.yaml');
        })
        .command(['new', '--no-tty', '-n=specification.yaml'])
        .it('runs new command', ctx => {
            expect(ctx.stdout).to.contain('specification.yaml');
        });
});

image

only you can make sure that specification.yaml does not exist when you are running the test.
I am still trying to figure out how the prompt testing works and I will let you know when I get something.

test/testHelper.ts Outdated Show resolved Hide resolved
@Samridhi-98 Samridhi-98 requested a review from Souvikns February 6, 2022 14:35
test/commands/new.test.ts Outdated Show resolved Hide resolved
@Samridhi-98 Samridhi-98 requested a review from Souvikns February 12, 2022 13:19
Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

Wow this is interesting, the tests are running in my local dev environment, but looking into the logs I think it is not waiting for the command to finish and thus is failing and some times it is causing problem for other tests as well.

 runs new command:
     AssertionError: expected '' to include 'specification.yaml'
      at Context.<anonymous> (test/commands/new.test.ts:9:29)
      at Object.run (node_modules/fancy-test/lib/base.js:44:38)
      at Context.run (node_modules/fancy-test/lib/base.js:68:36)

test/commands/new.test.ts Outdated Show resolved Hide resolved
@Samridhi-98 Samridhi-98 requested a review from Souvikns February 14, 2022 15:48
@Souvikns
Copy link
Member

image

The test is flaky, sometimes it is running, and sometimes not.

.stdout()
.command(['new', '--no-tty', '-n=specification.yaml'])
.it('runs new command', (ctx, done) => {
expect(ctx.stdout).to.contain('specification.yaml');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect(ctx.stdout).to.contain('specification.yaml');
expect(ctx.stderr).to.equals('');
expect(ctx.stdout).contains('specification.yaml');

This is working for me, you can try this once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still showing error 😟😟

image

Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

Try what I suggested once, and if it does not work let me know I will open a PR to your branch 👍🏻

@Samridhi-98 Samridhi-98 requested a review from Souvikns February 27, 2022 06:44
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@Souvikns Souvikns left a comment

Choose a reason for hiding this comment

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

Looks good to me, I think it is ready to be merged 🚀

@Souvikns
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 4d8bb86 into asyncapi:master Feb 28, 2022
@derberg
Copy link
Member

derberg commented Mar 1, 2022

@all-contributors please add @Samridhi-98 for tests

@allcontributors
Copy link
Contributor

@derberg

I've put up a pull request to add @Samridhi-98! 🎉

@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 0.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Souvikns added a commit to Souvikns/cli that referenced this pull request Mar 8, 2022
* docs: fix missing toc (asyncapi#230)

* feat: add global specification watcher (asyncapi#220)

Co-authored-by: Lukasz Gornicki <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>

* chore(release): v0.14.0 (asyncapi#237)

* docs: add imabp as a contributor for code (asyncapi#239)

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* fix: update @asyncapi/studio to 0.10.0 version (asyncapi#241)

* chore(release): v0.14.1 (asyncapi#242)

* feat: added testing for new (asyncapi#212)

* docs: add Samridhi-98 as a contributor for test (asyncapi#248)

* ci: update global workflows (asyncapi#246)

* chore(release): v0.15.0 (asyncapi#249)

* feat: migrate to latest oclif version (asyncapi#203)

* chore: update bot name from codeowners file (asyncapi#247)

* ci: update global workflows (asyncapi#250)

* Create test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

Co-authored-by: Abir <[email protected]>
Co-authored-by: Lukasz Gornicki <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>
Co-authored-by: asyncapi-bot <[email protected]>
Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Samriddhi <[email protected]>
Co-authored-by: asyncapi-bot <[email protected]>
Souvikns added a commit to Souvikns/cli that referenced this pull request Mar 8, 2022
* docs: fix missing toc (asyncapi#230)

* feat: add global specification watcher (asyncapi#220)

Co-authored-by: Lukasz Gornicki <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>

* chore(release): v0.14.0 (asyncapi#237)

* docs: add imabp as a contributor for code (asyncapi#239)

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>

* fix: update @asyncapi/studio to 0.10.0 version (asyncapi#241)

* chore(release): v0.14.1 (asyncapi#242)

* feat: added testing for new (asyncapi#212)

* docs: add Samridhi-98 as a contributor for test (asyncapi#248)

* ci: update global workflows (asyncapi#246)

* chore(release): v0.15.0 (asyncapi#249)

* feat: migrate to latest oclif version (asyncapi#203)

* chore: update bot name from codeowners file (asyncapi#247)

* ci: update global workflows (asyncapi#250)

* Create test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

* Update test.yml

Co-authored-by: Abir <[email protected]>
Co-authored-by: Lukasz Gornicki <[email protected]>
Co-authored-by: Maciej Urbańczyk <[email protected]>
Co-authored-by: asyncapi-bot <[email protected]>
Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
Co-authored-by: Samriddhi <[email protected]>
Co-authored-by: asyncapi-bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for "new" and "start studio" commands
5 participants