Skip to content

test: adding test coverage for modeler, plotting, logger, and parameters #2108

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

syscordan
Copy link
Contributor

@syscordan syscordan commented Jul 14, 2025

Description

adding test coverage for modeler, plotting, logger, and parameters

Checklist

  • I have tested my changes locally.
  • I have added necessary documentation or updated existing documentation.
  • I have followed the coding style guidelines of this project.
  • I have added appropriate unit tests.
  • I have reviewed my changes before submitting this pull request.
  • I have linked the issue or issues that are solved to the PR if any.
  • I have assigned this PR to myself.
  • I have added the minimum version decorator to any new backend method implemented.
  • I have made sure that the title of my PR follows Conventional commits style (e.g. feat: extrude circle to cylinder)

@syscordan syscordan self-assigned this Jul 14, 2025
@syscordan syscordan added the testing Anything related to tests label Jul 14, 2025
@syscordan syscordan requested a review from a team as a code owner July 14, 2025 14:00
Comment on lines 73 to 94
def test_adding_to_plotter(modeler: Modeler, verify_image_cache):
"""Testing out adding things to an existing plotter while changing the color and is suppressed"""
plotter = GeometryPlotter(allow_picking=True)
plane = Plane(origin=[0, 0, 0], direction_x=[1, 0, 0], direction_y=[0, 1, 0])
box_plane = Sketch(plane=plane)
plotting_options = {"clipping_plane": True}
plotter.add_sketch(box_plane, show_plane=True, show_frame=True, **plotting_options)
design = modeler.create_design("Box")
box_plane.box(Point2D([0.0, 0.0]), width=1, height=1)
box = design.extrude_sketch("Box", box_plane, 1)
box_plot = MeshObjectPlot(box, mesh=None)
plotter.add_body_edges(box_plot)
box.set_suppressed(True)
plotter.add_body(box)
box.set_suppressed(False)
plotter.add_body(box)
plotter.add_face(box.faces[0])
plotter2 = GeometryPlotter(allow_picking=True, use_service_colors=True)
box.faces[0].color = "blue"
plotter2.add_face(box.faces[0])
plotter.show(plotting_object=box)

Copy link
Member

Choose a reason for hiding this comment

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

This test needs to include more clarifications of what we are trying to attempt. Also I see 2 separate plotters. Graphics tests should ideally contain one only. We should also create a screenshot so that pytest-pyvista plugin can verify that the screenshot is the same as the reference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created two tests, one for each plotter. Please let me know if I did the screenshots incorrectly

Comment on lines 962 to 979
"""@skip_no_xserver
def test_export_glb_nofilename(modeler: Modeler, verify_image_cache):
Test exporting a box to glb.
# Create a Sketch
sketch = Sketch()
sketch.box(Point2D([10, 10], UNITS.mm), Quantity(10, UNITS.mm), Quantity(10, UNITS.mm))

# Create your design on the server side
design = modeler.create_design("GLBBoxNoName")

# Extrude the sketch to create a body
box_body = design.extrude_sketch("JustABoxNoName", sketch, Quantity(10, UNITS.mm))

pl = GeometryPlotter()
pl.plot(box_body)

pl.export_glb(filename=None)"""

Copy link
Member

Choose a reason for hiding this comment

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

Is this commented out test intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it

Comment on lines 999 to 1001

pl.export_glb(filename=None)

Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for adding this export? Just to verify the default name? We should verify that the file exists in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, to verify the default name if none is given

@@ -38,6 +40,63 @@
LOG_LEVELS = {"CRITICAL": 50, "ERROR": 40, "WARNING": 30, "INFO": 20, "DEBUG": 10}


def test_add_instance():
# Testing adding an instance logger while checking if log has certain key
Copy link
Member

Choose a reason for hiding this comment

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

Docstrings for defining the tests should go with """content""" instead of # content


def test_custom_and_child_log():
# Testing out writing a child log and adding std handler to it
custom_filename = os.path.join(os.getcwd(), "custom_geometry.log")
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using the os library for path handling, please use pathlib and Path objects

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

Successfully merging this pull request may close these issues.

3 participants