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

Move integration tests from test/functional/ to test/integration/ #10800

Merged
merged 3 commits into from Aug 4, 2017
Merged

Move integration tests from test/functional/ to test/integration/ #10800

merged 3 commits into from Aug 4, 2017

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Aug 4, 2017

All these tests require the AMP runtime to be built, and need a full gulp build before they can run. Therefore, they are integration tests, and should be moved from test/functional to test/integration, or in the case of extensions, from test/ to test/integration. After this PR is merged, #10792 will make it so that we only run gulp css prior to running the unit tests on Travis. The following will now work:

gulp clean
gulp css
gulp test --nobuild --unit

To be followed by #10792
Partial fix to #10785

@rsimha
Copy link
Contributor Author

rsimha commented Aug 4, 2017

/to @choumx @lannka

@rsimha
Copy link
Contributor Author

rsimha commented Aug 4, 2017

/cc @cramforce

@rsimha
Copy link
Contributor Author

rsimha commented Aug 4, 2017

Step 1 successful: All unit tests now run with just gulp css. See https://gist.github.com/rsimha-amp/c635de4c8842ba9106b98b759578c148

Coming up: Fix presubmit and integration test errors.

Copy link

@dreamofabear dreamofabear left a comment

Choose a reason for hiding this comment

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

Nice! Thought there would be more problematic tests.

@rsimha
Copy link
Contributor Author

rsimha commented Aug 4, 2017

@choumx, yeah, I was surprised. Let's not celebrate too early though.

Step 2 successful: Presubmit errors fixed.

Step 3 is interesting: There are now a bunch of failures in the tests that were moved to /integration. https://travis-ci.org/ampproject/amphtml/jobs/261080620 Not sure if these are due to moving the files, or bugs that always existed in the tests that are now being exposed on other browsers due to --saucelabs.

@rsimha
Copy link
Contributor Author

rsimha commented Aug 4, 2017

Step 3 successful: https://gist.github.com/rsimha-amp/fe64bb089593002207a0c28e6ff4c949

Will merge this as soon as Travis goes green.

@rsimha rsimha merged commit 009bc86 into ampproject:master Aug 4, 2017
@rsimha rsimha deleted the 2017-08-04-MoveIntegrationTests branch August 4, 2017 20:33
@dreamofabear
Copy link

@rsimha-amp I think there was some miscommunication -- we should still be running these moved integration tests on Chrome at least. After commit 36b70ab, we're now skipping a lot of integration tests that were previously running on Travis (latest Chrome).

@rsimha
Copy link
Contributor Author

rsimha commented Aug 7, 2017

Aha, they were previously being run on latest Chrome due to the unqualified gulp test but now, they're being filtered out on one shard due to --unit, and on the other shard due to --saucelabs. (I was under the false impression that skipSauceLabs would still run the tests on latest Chrome, but that's not true.)

I could remove the --unit from the first shard, but that would undo all the speed gains due to not building before the unit tests. Is there an alternative to skipSauceLabs that runs an integration test only on latest Chrome?

I'll send you a PR shortly to rectify this.

@rsimha
Copy link
Contributor Author

rsimha commented Aug 7, 2017

@choumx, sent you #10825.

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.

None yet

4 participants