Problem/Motivation

To ensure the module works as expected and doesn't break on changes, we should have general tests for the basic functionalities.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork commerce_product_menu_ui-3302659

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue tags: +Novice

I think this could be a nice novice task to learn writing tests in Drupal?

sapnil_biswas made their first commit to this issue’s fork.

sapnil_biswas opened merge request !10

sapnil_biswas’s picture

sapnil_biswas commented
Status: Active » Needs review

Hi, I’ve implemented the basic functionality tests as discussed and opened MR !10.
This is my first time working on tests in Drupal, so I’d really appreciate any feedback or suggestions for improvement.
Marking this as Needs Review. Thanks in advance!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil
sapnil_biswas’s picture

sapnil_biswas commented

Thanks for the updates and improvements — this really helped me better understand the expected structure.
Please let me know if there’s anything else I should update or improve on my side.

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs review » Needs work
sapnil_biswas’s picture

sapnil_biswas commented
Status: Needs work » Needs review

Tried fixing the review comments left, this is one of my first contributions so please let me know if there are changes required

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

Thanks, still tests are broken

sapnil_biswas’s picture

sapnil_biswas commented
Status: Needs work » Needs review

addressed the review comments and made an effort to keep the scope limited to the fundamental functionality tests and the most recent pipeline for 44364a3 is passing. I'm moving this back to review, but if you would like the remaining CI-support changes to be further divided, please let me know.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil

Nice, it's green! @grevil please review.

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs review » Needs work
sapnil_biswas’s picture

sapnil_biswas commented
Status: Needs work » Needs review

Tried resolving the comments marking this as review please let me know if any further changes or rectification required

grevil’s picture

grevil commented
Status: Needs review » Reviewed & tested by the community

Thanks, LGTM! Once the tests are green, we can merge this.

sapnil_biswas’s picture

sapnil_biswas commented

Thanks @grevil, @Anybody it was great learning experience working on this issue

grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed

Thanks for working on this @sapnil_biswas!

Merging.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.