Problem/Motivation

I noticed that ModalPageHelperService class needs unit test. I'd like to try to work on it.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 3306890-14.patch6.32 KBsophiavs
#13 interdiff_10-12.txt538 bytessophiavs
#12 interdiff_10-12.patch538 bytessophiavs
#12 3306890-12.patch0 bytessophiavs
#10 interdiff_07-10.txt1019 byteslucassc
#10 3306890-10.patch6.32 KBlucassc
#7 interdiff_06-07.txt4.67 KBgquisini
#7 3306890-07.patch6.19 KBgquisini
#6 interdiff_04-06.txt6.21 KBlucassc
#6 3306890-06.patch6.41 KBlucassc
#5 interdiff_04-05.txt18.34 KBlucassc
#5 3306890-05.patch18.8 KBlucassc
#4 3306890-04.patch2.27 KBlucassc

Comments

gquisini created an issue. See original summary.

gquisini’s picture

gquisini commented
Issue summary: View changes
gquisini’s picture

gquisini commented
Assigned: Unassigned » gquisini
lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
Status: Active » Needs work
StatusFileSize
new 3306890-04.patch2.27 KB

Hi!

Submitting new patch with mocks and a ModalPageHelperService object.

Edit: sorry but I submitted an outdated patch. @gquisini already got it right, we'll send a new patch soon.

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
StatusFileSize
new 3306890-05.patch18.8 KB
new interdiff_04-05.txt18.34 KB

Added tests for:

  • verifyIfUserHasAccessOnModal
  • verifyIfModalIsAvailableForEveryone
  • testGetModalOptions
lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
StatusFileSize
new 3306890-06.patch6.41 KB
new interdiff_04-06.txt6.21 KB

Once more I submitted the wrong path (not my day e.e). Please, ignore files in #5.

Submitting the correct patch and interdiff.

gquisini’s picture

gquisini commented
StatusFileSize
new 3306890-07.patch6.19 KB
new interdiff_06-07.txt4.67 KB

In this patch, we created some tests to:
- verifyIfUserHasAccessOnModal()
- verifyIfModalIsAvailableForEveryone()
- getModalOptions()

gquisini’s picture

gquisini commented
Assigned: gquisini » Unassigned
Status: Needs work » Needs review

Changing status to "needs review" because we're starting to feel stuck.
This is our first unit test, so we may have made some mistakes. Feel free to suggest modifications and improvements, we'll be willing to fix them out.
Kindly review.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Needs review » Needs work

As the $this->setUp(); method is called, can we move the tests bellow to a new test function?

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
Status: Needs work » Needs review
StatusFileSize
new 3306890-10.patch6.32 KB
new interdiff_07-10.txt1019 bytes

Good idea, @paulocs!

sophiavs’s picture

sophiavs commented

I'll do the review

sophiavs’s picture

sophiavs commented
StatusFileSize
new 3306890-12.patch0 bytes
new interdiff_10-12.patch538 bytes

I reviewed the concepts of UnitTests and everything seems to be right.
I found one coding standards error on a "true" value that should be "TRUE",

sophiavs’s picture

sophiavs commented
StatusFileSize
new interdiff_10-12.txt538 bytes

Sorry i uploaded the interdiff as a .patch instead of .txt

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Needs review » Needs work

Patch Failed to Apply. Moving to NW.

sophiavs’s picture

sophiavs commented
StatusFileSize
new 3306890-14.patch6.32 KB

I saw now that the patch was blank

lucassc’s picture

lucassc
he/him
Primary language Portuguese, Brazil
Location Rio de Janeiro
commented
Status: Needs work » Needs review
lucasbaralm’s picture

lucasbaralm
He/Him
commented
Assigned: Unassigned » lucasbaralm

I will review this.

lucasbaralm’s picture

lucasbaralm
He/Him
commented
Assigned: lucasbaralm » Unassigned
Status: Needs review » Reviewed & tested by the community

As @sophiavs said, all tests are right, covering all the methods of the ModalPageHelperService class that need to be tested. With the patch from #15 that are no standard issues remaining and as all unit tests work i'm moving to RTBC.

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Parent issue: » #3307178: Plan to next Modal's release

  • 04f92a9 committed on 4.1.x 
    Issue #3306890 by lucassc, sophiavs, gquisini, paulocs, lucasbaralm: Add...
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
commented
Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

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