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

Run Dart VM tests with snapshot #73

Merged
merged 2 commits into from Aug 2, 2019

Conversation

@jathak
Copy link
Member

commented Aug 2, 2019

Similar to how the Node tests require pub run grinder js to be run, the Dart VM tests now require pub run grinder app-snapshot to be run, drastically decreasing the amount of time it takes to run all of the tests.

The new snapshot tag is necessary for these tests now since filtering with -t node will still cause the check for the snapshot's existence to be run, but -x snapshot won't.

Run Dart VM tests with snapshot
Similar to how the Node tests require pub run grinder js to be run, the
Dart VM tests now require pub run grinder app-snapshot to be run,
drastically decreasing the amount of time it takes to run all of the
tests.

The new `snapshot` tag is necessary for these tests now since filtering
with `-t node` will still cause the check for the snapshot's existence
to be run, but `-x snapshot` won't.

@jathak jathak force-pushed the snapshot-test branch from d8b9d32 to 46c0612 Aug 2, 2019

@jathak jathak requested a review from nex3 Aug 2, 2019

@nex3

nex3 approved these changes Aug 2, 2019

Copy link
Contributor

left a comment

It's definitely a good idea to build a snapshot on CI, but when you're iterating on a test you may also want to be able to run directly from source. In Dart Sass, we have a check that the snapshot is up-to-date if it exists, but we don't complain if it doesn't. Up to you, though.

@@ -89,7 +91,7 @@ jobs:
if: *deploy-if
env:
# PUB_CREDENTIALS="..."
- secure: "QCNhSiluq2TPf20ekwWlf+TkyzwnGwqS3pU+3XogRlP6ffRcG98zL9pn7cp8xmaM50x9PY6nk0t+oxkiOFw7sKhECfIRv0Z+P4cURhTUJ25+FqLPupCSqehoF93Bx8fXQR+2KCEP+cEeu2dcDU0hAZnzSWliRm/X/6Hfy+2M5eVc4x0+YC/gDjdbNbgH5P29qhRSB7klB8FtKDQdmQro0l1jSe69RGUmaeFSG7ryn+OlCJNMZqwZBd2Yb7pShiegU1ks/fIrC+pf/Yx2/VUscyLGwWqNxl6xXeZko3bNNCNPhPNIjArZJTNKMHvuIvKEvjfrUIr6TwGsw9vJmGjAbSiZzeXfeJ5zBp9ZaL1i8g5WwvVoQcbFgTn/vCisLZ97NP+0ipKr1VXxDfJ+MThaFsbOnl4wcC9XrrqQERWjH4lNGdfywvjql+d1M48veGqeYYY64UhiGDS+98t6GG+Q+ScQh5LDbn/uaQhi6DUCCqzaphdr78ylVrt7RzIdauOcu/CY3q22Kfm+wOZd+4S3Abk3L7P3eWl60sm/yUCx6yTA1rZPBylL5+R3cQS+9Lp9lll03Xcuf/L+MD7pVsN/RB5L52zn51oxtuVvjU01R0JZ8vWSgZ7CMs1BD7DXCjz9deGvBCHJR5aFelygt9ZSuIaM2xpZ2uSFscUnf6qjSh4="
- secure: "QCNhSiluq2TPf20ekwWlf+TkyzwnGwqS3pU+3XogRlP6ffRcG98zL9pn7cp8xmaM50x9PY6nk0t+oxkiOFw7sKhECfIRv0Z+P4cURhTUJ25+FqLPupCSqehoF93Bx8fXQR+2KCEP+cEeu2dcDU0hAZnzSWliRm/X/6Hfy+2M5eVc4x0+YC/gDjdbNbgH5P29qhRSB7klB8FtKDQdmQro0l1jSe69RGUmaeFSG7ryn+OlCJNMZqwZBd2Yb7pShiegU1ks/fIrC+pf/Yx2/VUscyLGwWqNxl6xXeZko3bNNCNPhPNIjArZJTNKMHvuIvKEvjfrUIr6TwGsw9vJmGjAbSiZzeXfeJ5zBp9ZaL1i8g5WwvVoQcbFgTn/vCisLZ97NP+0ipKr1VXxDfJ+MThaFsbOnl4wcC9XrrqQERWjH4lNGdfywvjql+d1M48veGqeYYY64UhiGDS+98t6GG+Q+ScQh5LDbn/uaQhi6DUCCqzaphdr78ylVrt7RzIdauOcu/CY3q22Kfm+wOZd+4S3Abk3L7P3eWl60sm/yUCx6yTA1rZPBylL5+R3cQS+9Lp9lll03Xcuf/L+MD7pVsN/RB5L52zn51oxtuVvjU01R0JZ8vWSgZ7CMs1BD7DXCjz9deGvBCHJR5aFelygt9ZSuIaM2xpZ2uSFscUnf6qjSh4="

This comment has been minimized.

Copy link
@nex3

nex3 Aug 2, 2019

Contributor

What changed here?

This comment has been minimized.

Copy link
@jathak

jathak Aug 2, 2019

Author Member

If I ignore whitespace with git diff -w, this goes away, so I think my editor automatically converted tabs to spaces.

.travis.yml Outdated
@@ -27,11 +29,11 @@ jobs:

# Testing on Node.js
- env: NODE_VERSION=stable
dart_task: {test: -t node}
dart_task: {test: -x snapshot}

This comment has been minimized.

Copy link
@nex3

nex3 Aug 2, 2019

Contributor

Why run non-Node tests here and below?

This comment has been minimized.

Copy link
@jathak

jathak Aug 2, 2019

Author Member

I forgot about the Patch tests and was thinking that every test was either tagged node or snapshot. Making the snapshot optional eliminated this need for this tag, so I changed it back.

@jathak

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2019

Okay, I changed it to only use the snapshot if it exists and is up-to-date and to fallback to running directly from source if not.

@jathak jathak merged commit 5193e7b into master Aug 2, 2019

1 check passed

Travis CI - Pull Request Build Passed
Details

@jathak jathak deleted the snapshot-test branch Aug 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.