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

Allow to upload results to LH CI server #20

Open
szimek opened this issue Sep 3, 2021 · 3 comments · May be fixed by #22
Open

Allow to upload results to LH CI server #20

szimek opened this issue Sep 3, 2021 · 3 comments · May be fixed by #22

Comments

@szimek
Copy link

szimek commented Sep 3, 2021

Hey!

I've just set up this action for our repo and it works great. Thanks!

It would be great to be able to track how these metrics change over time, so I started looking into LH CI server. Would it be enough to allow one to set LH CI server URL and token and then change the generated lighthouserc.yml to include this config:

upload: {
  target: 'lhci',
  serverBaseUrl: 'https://your-lhci-server-url.example.com',
  token: 'Your *build token* goes here', // could also use LHCI_TOKEN variable instead
},

if LH CI server URL and token are provided? If so, I could prepare a PR.

@charlespwd
Copy link
Collaborator

Hey! I think that'd be great but I would suggest offering a way to "merge" the configs instead. Our lighthouserc.yml right now is pretty barebones. We generate it for the following reason:

  1. Wanted to make the important easy
  2. Needed to set the puppeteer launch options and the puppeteer script to be able to run on development themes (the hardest part to get right on your own).

But we definitely are lacking when it comes to "Make the important things easy, everything else possible." The everything else possible part we overlooked 🙃

What would you think of offering an option to merge the configs somehow (I think the one in here should be the "default" and the one a user provide would be overrides)? Is that something you'd be interested in doing?

@szimek
Copy link
Author

szimek commented Oct 18, 2021

Hey!

I just encountered another issue with this action and then remembered about this issue :)

Would it be ok to use yq to merge YAML configs? E.g. one could provide their own custom config like this (not sure about the naming here):

name: Shopify Lighthouse CI
on: [push]
jobs:
  lhci:
    name: Lighthouse
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Lighthouse
        uses: shopify/lighthouse-ci-action@1.0
        with:
          app_id: ${{ secrets.SHOP_APP_ID }}
          app_password: ${{ secrets.SHOP_APP_PASSWORD }}
          store: ${{ secrets.SHOP_STORE }}
          custom_config:
            ci:
              upload:
                target: 'lhci'
                serverBaseUrl: 'https://your-lhci-server-url.example.com'
                token: 'Your *build token* goes here'

The default lighthouserc.yml would be created as it is now, value of INPUT_CUSTOM_CONFIG would be stored in another YAML file (I'm not sure if GitHub Actions will correctly convert YAML code into an env variable value...) and finally these 2 files could be merged with yq:

yq eval-all 'select(fileIndex == 0) * select(filename == "lighthouserc-custom.yml")' lighthouserc.yml lighthouserc-custom.yml -i

Would something like this work?

@szimek szimek linked a pull request Oct 25, 2021 that will close this issue
3 tasks
@alexantom
Copy link

alexantom commented Apr 18, 2022

if you allow these ENV variables part of the shell script, we will able to send data to custom lighthouse server
@charlespwd

LHCI_TOKEN
LHCI_SERVER_BASE_URL
LHCI_TARGET
[[ -n "$INPUT_LHCI_TOKEN" ]]    && export LHCI_TOKEN="$INPUT_LHCI_TOKEN"
[[ -n "$INPUT_ LHCI_SERVER_BASE_URL" ]] && export LHCI_SERVER_BASE_URL="$INPUT_LHCI_SERVER_BASE_URL"
[[ -n "$INPUT_ LHCI_TARGET" ]]        && export LHCI_TARGET="$INPUT_LHCI_TARGET"

alexantom@c896983

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants