Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

Emitter tests #168

Merged
merged 14 commits into from
May 5, 2021
Merged

Emitter tests #168

merged 14 commits into from
May 5, 2021

Conversation

alfredclwong
Copy link
Collaborator

Changes

  • Bug fix: calling AngularPower.Cosine() caused an UndefVarError
  • simplified secondary constructors in Directions.jl
  • code style improvements
  • generateray -> generate (pls review: 88856f5)
  • test coverage for Emitters module

@alfredclwong alfredclwong added bug Something isn't working testing Adding missing tests or correcting existing tests labels Apr 30, 2021
@alfredclwong alfredclwong requested review from BrianGun and galran April 30, 2021 13:19
@alfredclwong alfredclwong self-assigned this Apr 30, 2021
@alfredclwong alfredclwong enabled auto-merge (squash) April 30, 2021 13:23
@alfredclwong alfredclwong force-pushed the alfred/emitter-tests branch from 9b80c47 to 61c85a6 Compare April 30, 2021 13:40
@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2021

Codecov Report

Merging #168 (b67b80a) into main (dd075c1) will increase coverage by 1.95%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #168      +/-   ##
==========================================
+ Coverage   50.82%   52.78%   +1.95%     
==========================================
  Files          66       64       -2     
  Lines        6467     6426      -41     
==========================================
+ Hits         3287     3392     +105     
+ Misses       3180     3034     -146     
Impacted Files Coverage Δ
src/Optical/Emitters/AngularPower.jl 100.00% <100.00%> (+84.61%) ⬆️
src/Optical/Emitters/Directions.jl 100.00% <100.00%> (+94.52%) ⬆️
src/Optical/Emitters/Origins.jl 100.00% <100.00%> (+94.23%) ⬆️
src/Optical/Emitters/Sources.jl 95.89% <100.00%> (+43.83%) ⬆️
src/Optical/Emitters/Spectrum.jl 17.50% <100.00%> (+12.23%) ⬆️
src/Geometry/Primitives/Qtype.jl 75.00% <0.00%> (-8.34%) ⬇️
src/Optical/LensAssembly.jl 48.93% <0.00%> (-7.24%) ⬇️
src/Optical/OpticalInterface.jl 63.23% <0.00%> (-4.30%) ⬇️
src/Geometry/CSG/Intersection.jl 48.57% <0.00%> (-2.14%) ⬇️
src/Geometry/Geometry.jl 0.00% <0.00%> (-1.86%) ⬇️
... and 14 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 dd075c1...b67b80a. Read the comment docs.

@alfredclwong alfredclwong force-pushed the alfred/emitter-tests branch from 61c85a6 to 4f03a51 Compare April 30, 2021 14:25
@BrianGun
Copy link
Contributor

BrianGun commented May 2, 2021

are we just waiting for @galran to approve the getindex change? Once that is approved can we approve the PR?

@alfredclwong
Copy link
Collaborator Author

are we just waiting for @galran to approve the getindex change? Once that is approved can we approve the PR?

Confirmation would be nice - I'll ask today at the meeting. It's most likely correct anyway, but good to be safe.

@alfredclwong alfredclwong disabled auto-merge May 4, 2021 10:10
@alfredclwong alfredclwong force-pushed the alfred/emitter-tests branch from b67b80a to 1f855fb Compare May 4, 2021 10:14
@galran
Copy link
Collaborator

galran commented May 5, 2021

looks ok to me.
i'm ashamed to say it but i don't remember how to "officially" approve it :)

@alfredclwong
Copy link
Collaborator Author

looks ok to me.
i'm ashamed to say it but i don't remember how to "officially" approve it :)

It's pretty obscure - you have to go to "Files changed", click "Review changes", select "Approve", and then "Submit review". You can also use the other options to create review comments without approving the PR.

@alfredclwong alfredclwong requested a review from friggog May 5, 2021 08:52
@alfredclwong alfredclwong enabled auto-merge (squash) May 5, 2021 08:53
@alfredclwong alfredclwong merged commit e0204ee into main May 5, 2021
@alfredclwong alfredclwong deleted the alfred/emitter-tests branch May 5, 2021 10:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working testing Adding missing tests or correcting existing tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants