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

test(docs-infra): add missing tests for observables and promises #34537

Closed
wants to merge 3 commits into from

Conversation

sonukapoor
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #34398

Does this PR introduce a breaking change?

  • Yes
  • No

@sonukapoor sonukapoor requested review from IgorMinar and a team as code owners December 22, 2019 23:27
@gkalpak gkalpak added comp: docs-infra action: review The PR is still awaiting reviews from at least one requested reviewer state: blocked target: patch This PR is targeted for the next patch release labels Dec 23, 2019
@ngbot ngbot bot modified the milestone: needsTriage Dec 23, 2019
@gkalpak
Copy link
Member

gkalpak commented Dec 23, 2019

Marking this as blocked on #34327, since it builds on top of that.

@sonukapoor
Copy link
Contributor Author

Marking this as blocked on #34327, since it builds on top of that.

@gkalpak This is unblocked now since #34327 has been merged. Can you review it please?

@gkalpak gkalpak changed the title Tests for observables test(docs-infra): add missing tests for obserables and promises Jan 22, 2020
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Thx, @sonukapoor ❤️
I've left some comments. Also, the current commit message ("tests for observables and promises") is too generic. Can you, please, change it to be more specific. E.g. something like:

test(docs-infra): add tests for `comparing-observables` example

@gkalpak
Copy link
Member

gkalpak commented Jan 23, 2020

I, also, just realized that we need to make sure the tests are run on CI (currently they are not afaict) 😁

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Just few more (minor) comments to go 😃

Also, as mentioned in #34537 (comment), we need to run these tests on CI. This is similar to what you did for other Observables-related examples in #34063. I.e. you need to:

@sonukapoor
Copy link
Contributor Author

Just few more (minor) comments to go 😃

Also, as mentioned in #34537 (comment), we need to run these tests on CI. This is similar to what you did for other Observables-related examples in #34063. I.e. you need to:

Yes, I haven't gotten to that yet. I wanted to make sure that we are happy with the codebase now. I should be able to look into the testing from CI next.

@sonukapoor sonukapoor requested a review from a team as a code owner January 25, 2020 22:47
@sonukapoor sonukapoor force-pushed the tests-for-observables branch 4 times, most recently from 232d2e0 to a5231d7 Compare January 26, 2020 00:14
@mary-poppins
Copy link

You can preview a5231d7 at https://pr34537-a5231d7.ngbuilds.io/.

@mary-poppins
Copy link

You can preview d2bd1cc at https://pr34537-d2bd1cc.ngbuilds.io/.

@mary-poppins
Copy link

You can preview f3bd82e at https://pr34537-f3bd82e.ngbuilds.io/.

@mary-poppins
Copy link

You can preview c265a4f at https://pr34537-c265a4f.ngbuilds.io/.

This commit adds missing tests for obserables and promises which
are both stand-alone mini-apps.
This commit adds the necessary custom commands to run the tests in a
node environment.
@mary-poppins
Copy link

You can preview e25f686 at https://pr34537-e25f686.ngbuilds.io/.

@mary-poppins
Copy link

You can preview ec9406f at https://pr34537-ec9406f.ngbuilds.io/.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Nice job, @sonukapoor! Thx 👍

Reviewed-for: global-docs-approvers

@gkalpak gkalpak removed the request for review from alxhub August 27, 2020 15:29
@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Aug 27, 2020
josephperrott pushed a commit that referenced this pull request Aug 27, 2020
This commit adds missing tests for obserables and promises which
are both stand-alone mini-apps.

PR Close #34537
josephperrott pushed a commit that referenced this pull request Aug 27, 2020
This commit adds the necessary custom commands to run the tests in a
node environment.

PR Close #34537
josephperrott pushed a commit that referenced this pull request Aug 27, 2020
This commit adds the necessary custom commands to run the tests in a
node environment.

PR Close #34537
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this pull request Aug 29, 2020
…lar#34537)

This commit adds missing tests for obserables and promises which
are both stand-alone mini-apps.

PR Close angular#34537
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this pull request Aug 29, 2020
This commit adds the necessary custom commands to run the tests in a
node environment.

PR Close angular#34537
subratpalhar92 added a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this pull request Aug 31, 2020
subratpalhar92 added a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this pull request Aug 31, 2020
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this pull request Aug 31, 2020
…lar#34537)

This commit adds missing tests for obserables and promises which
are both stand-alone mini-apps.

PR Close angular#34537
subratpalhar92 pushed a commit to SUBRATPALHAR-ALL-JAVASCRIPT/angular that referenced this pull request Aug 31, 2020
This commit adds the necessary custom commands to run the tests in a
node environment.

PR Close angular#34537
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
…lar#34537)

This commit adds missing tests for obserables and promises which
are both stand-alone mini-apps.

PR Close angular#34537
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
This commit adds the necessary custom commands to run the tests in a
node environment.

PR Close angular#34537
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker aio: preview cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants