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

add service number #233

Merged
merged 1 commit into from Aug 12, 2020
Merged

add service number #233

merged 1 commit into from Aug 12, 2020

Conversation

caleb15
Copy link
Contributor

@caleb15 caleb15 commented Aug 5, 2020

I think CIRCLE_WORKFLOW_ID maps to the number variable (aka service_number) but I'm not 100% sure.
Note that node coveralls uses CIRCLE_WORKFLOW_ID as the service number. See https://github.com/nickmerwin/node-coveralls/blob/705c3b5963e3cc76f8e70381dbae20979008c9b8/lib/getOptions.js#L66

@caleb15 caleb15 marked this pull request as draft August 5, 2020 21:58
@caleb15 caleb15 changed the title WIP add service number add service number Aug 5, 2020
@caleb15
Copy link
Contributor Author

caleb15 commented Aug 5, 2020

Circleci pre-commit failed because of pylint but that is a unrelated to my change.

pylint...................................................................Failed
- hook id: pylint
- exit code: 16

@caleb15
Copy link
Contributor Author

caleb15 commented Aug 6, 2020

I got the parallel option working with using workflow id as the service num so I'm a bit more confident this is the right approach now:

CI Config:

# js tests:
      - run: yarn test --collectCoverage --coverageDirectory=ff/static/coverage --ci --maxWorkers=3 --reporters=default --reporters=jest-junit

      - run: export COVERALLS_PARALLEL=true;yarn coveralls -v < ff/static/coverage/lcov.info

# python tests (runs after js tests)
      - run:
          name: run tests
          command: venv/bin/coverage run --parallel-mode manage.py test --noinput --parallel=4 --verbosity=2
          no_output_timeout: 20m
      - run: venv/bin/coverage combine
      - run: venv/bin/coverage report
      - run: export COVERALLS_SERVICE_JOB_NUMBER=$CIRCLE_WORKFLOW_ID;export COVERALLS_PARALLEL=true;venv/bin/coveralls
      # coveralls --finish is broken so we need to specify job num
      - run: export COVERALLS_SERVICE_JOB_NUMBER=$CIRCLE_WORKFLOW_ID;venv/bin/coveralls --finish

@TheKevJames
Copy link
Owner

@caleb15 thanks for the contribution here -- just catching up on #234 and #235 now. Why did this PR get closed?

@caleb15
Copy link
Contributor Author

caleb15 commented Aug 11, 2020

@TheKevJames sorry about that, I accidentally closed it. In my PR https://github.com/15five/fifteen5/pull/30230 I wrote the following:

I experienced a bug in the coveralls python library so I raised a PR with a fix #233 and did export COVERALLS_SERVICE_JOB_NUMBER=$CIRCLE_WORKFLOW_ID; as a temporary workaround.

Github thought that me saying "fix #233" meant that the PR had been fixed and could be closed 🙄 Too smart for its own good!

@caleb15 caleb15 reopened this Aug 11, 2020
@TheKevJames
Copy link
Owner

Oh, wow, very weird that Github will close things cross-repos like that -- especially since you made the comment in a private repo and it closed a PR on an OSS repo owned by a completely different Github org. "Too smart for its own good!" no kidding!

Anyway, this changeset looks good to me -- I'm just going to wait for an answer on #235 to make sure it works for that case as well, then will merge and released this. Thanks for the contribution!

@TheKevJames TheKevJames merged commit 5e05654 into TheKevJames:master Aug 12, 2020
@caleb15 caleb15 deleted the patch-1 branch April 22, 2021 04:18
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 this pull request may close these issues.

None yet

2 participants