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

[ci] Update ci config to reduce javascript test time and some of cypress. #6016

Merged
merged 24 commits into from
Oct 4, 2018

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Oct 1, 2018

Highlight: Reduce time to run javascript test from 8-10mins to 3-5 mins

Details

  • Enable yarn and npm caches.
  • Use yarn install --frozen-lockfile instead of yarn install as recommended by travis.

javascript test

  • Use language: node_js for the javascript test instead of running it via python.
  • Remove npm run test because npm run cover already run the test. Basically we have been running js test twice.
  • Remove npm run build as the cypress test will always need to build. Seems redundant.

cypress test

  • No need to reinstall yarn as it is already available on travis box

others

  • Do not start services or run commands that are not necessary for the test. For example, start postgres only for the tests that need postgres.

@michellethomas @graceguo-supercat @mistercrunch @john-bodley

npm run build
npm run cypress run --record
npm run cypress run
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you removed --record? It doesn't work atm but I'm trying to get it to work, unless we think we don't want to use cypress dashboard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was trying to make it work via npm run cypress run -- --record then cypress complains asking for --key, which is required for recording. Will leave this for @michellethomas to handle in future review.

@codecov-io
Copy link

codecov-io commented Oct 1, 2018

Codecov Report

Merging #6016 into master will increase coverage by 13.25%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #6016       +/-   ##
===========================================
+ Coverage   64.63%   77.89%   +13.25%     
===========================================
  Files         445       46      -399     
  Lines       23764     9396    -14368     
  Branches     2639        0     -2639     
===========================================
- Hits        15361     7319     -8042     
+ Misses       8390     2077     -6313     
+ Partials       13        0       -13
Impacted Files Coverage Δ
superset/assets/src/components/Checkbox.jsx
...ets/src/dashboard/components/dnd/DragDroppable.jsx
superset/assets/src/components/EditableTitle.jsx
...src/explore/components/controls/VizTypeControl.jsx
...t/assets/src/components/InfoTooltipWithTrigger.jsx
.../src/dashboard/components/UndoRedoKeylisteners.jsx
...ts/src/explore/components/ExploreActionButtons.jsx
...sets/src/dashboard/components/DashboardBuilder.jsx
...et/assets/src/explore/components/RowCountLabel.jsx
superset/assets/src/CRUD/CollectionTable.jsx
... and 391 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 604524b...5741231. Read the comment docs.

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

This is huge! thanks for digging into this and optimizing + catching huge issues like running tests 2x 😮

@williaster williaster merged commit 50c701c into apache:master Oct 4, 2018
@kristw kristw deleted the kristw-ci branch October 4, 2018 22:01
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 12, 2018
…ess. (apache#6016)

* try travis config

* try more complex config

* fix config

* add cache and stages

* separate cache

* swap order

* fix redis

* remove env

* add cypress

* adjust caching

* reduce cache

* change order of execution

* change order again

* change node version

* install yarn via curl

* update cypress command

* update cypress command

* adjust cache and node version

* do not install yarn

* add npm cache

* remove export

* remove commented code

* change order of jobs

* fix indent
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants