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

Eliminate references to OTRS domains in the test suite #1013

Closed
bschmalhofer opened this issue May 22, 2021 · 31 comments · Fixed by #1200
Closed

Eliminate references to OTRS domains in the test suite #1013

bschmalhofer opened this issue May 22, 2021 · 31 comments · Fixed by #1200
Assignees
Labels
good first issue Good for newcomers tidying Tidying of the code
Milestone

Comments

@bschmalhofer
Copy link
Contributor

The command

$ grep -r "otrs.[com|org|de]" scripts/test/  | grep -v Copyright | grep -v bugs.otrs.org | wc -l
209

shows that OTRS domains are still referenced in the test scripts. These could be switched to OTOBO domains. Some test might also make DNS queries to OTRS domains.

@bschmalhofer bschmalhofer added good first issue Good for newcomers tidying Tidying of the code labels May 22, 2021
@bschmalhofer bschmalhofer added this to the OTOBO 10.0 milestone May 22, 2021
@svenoe
Copy link
Contributor

svenoe commented May 25, 2021

This has to dealt with on an individual basis (sometimes only checksums of the expected results would have to be recalculated, sometimes servers would have to be set up, which provide some API to communicate with,...), unfortunately, which was the reason that we left it untouched so far. Of course this should be corrected over time, but atm, I don't think the benefit justifies the time needed to at least completely do this.

@niccord
Copy link
Contributor

niccord commented May 28, 2021

I can work on this issue, if you want you can assign this to me.

How do you think this could be solved? Replacing "otrs.[com|org|de]" with "otobo.org"?

@bschmalhofer bschmalhofer changed the title Elimate reference to OTRS domains in the test suite Eliminate references to OTRS domains in the test suite May 28, 2021
@svenoe
Copy link
Contributor

svenoe commented May 28, 2021

Hi Nicola,

we would be happy if you do. :) You can just replace it with otobo.org, though I'm pretty sure that it won't be possible just like this everywhere, as I wrote before. But even if it can only be changed partly, if you help us and just rewrite some tests, which easily can be adapted to still run, it is helpful and welcome.

I will assign you for now - thanks, Sven

@bschmalhofer
Copy link
Contributor Author

@niccord The first step would be to verify that the unit tests, optimally including the Selenium, tests are succeeding in your devel environment. In case of stumbling blocks I could assist you with the setup.

@niccord
Copy link
Contributor

niccord commented May 31, 2021

@bschmalhofer I've launched the test suite including the Selenium part, but I am not sure about the result: I have a huge .out file (more than 500k rows), with lots of ok but with some error, too.

My environment is pretty clean and I've updated it from git before running the tests.

@bschmalhofer
Copy link
Contributor Author

@niccord yes, the test output is somewhat daunting. The relevant lines are the last two lines. There I have:

Files=951, Tests=84459, 5975 wallclock secs (30.69 usr  2.79 sys + 1503.09 cusr 180.52 csys = 1717.09 CPU)
Result: PASS

There are some 'not ok' cases, but these should be marked as TODO. Some sporadic failures are also tracked in #988. Don't worry about the error messages. They usually come from test code that deliberately provokes the errors.
If you have errors that are neither TODO nor covered in #988, then we would appreciate bug reports.

@niccord
Copy link
Contributor

niccord commented May 31, 2021

I have some MySQL table error, so I can't find that line in my test output. I try updating my docker container then run the tests again.

@niccord
Copy link
Contributor

niccord commented May 31, 2021

Is it expected for the test scripts to drop my database tables? Because for the second time in a row I found it that way 😨
(anyway, I still haven't the relevant lines you pointed out)

I have set this in my Kernel/Config.pm file:

# Test database used only when  is specified in the test file
$Self->{TestDatabase} = {
    DatabaseDSN  => "DBI:Pg:dbname=otobo_test;host=$Self->{DatabaseHost}",
    DatabaseUser => 'otrs',
    DatabasePw   => 'vRfmjkSijXuX1KKt',
};

but I also have:

$Self->{'TestHTTPHostname'} = 'localhost';

which is used by Selenium, I suppose.

I my test I found:

Here is a trace to the code that caused the context to be destroyed, this could
be an exit(), a goto, or simply the end of a scope:
Context destroyed at /opt/otobo_install/local/lib/perl5/Test2/Hub/Subtest.pm line 67.
...

@bschmalhofer
Copy link
Contributor Author

Hi @niccord ,
the database should no be wiped, but I wouldn't recommend to run the test suite on a database with useful data. I tend to start on a fresh install. I might have seen the 'Context destroyed' message at some time, but not lately.

I noticed that you are using Docker together with PostgreSQL. This is mostly untested from my side. In https://github.com/bschmalhofer/otobo-ideas/wiki/Running-the-Test-Suite I started to jot down the approach I usually use. Note that this should be simplified and eventually be turned into a nice tutorial. I hope it helps for getting started.

Best regards,
Bernhard

@niccord
Copy link
Contributor

niccord commented Jun 1, 2021

Hi Bernhard,
I am going to try with your approach, then.

Your tutorial seems simple and straightforward, thanks!

@niccord
Copy link
Contributor

niccord commented Jun 1, 2021

Since I am still unable to test with Selenium, I changed my approach: I have changed otrs with otobo as we said and then I launched all the test files I changed.

I will open a PR now, can you please test it with Selenium?

@bschmalhofer
Copy link
Contributor Author

Cool, I looked the changes and I notices some issues, mostly that Copyright notes were changed too. Could you take I look at the review comments in #1049 ?
In the meantime I try to provide a Docker setup that does not use HTTPS. This should simplify things a little bit.
RotherOSS/otobo-docker#53

@niccord
Copy link
Contributor

niccord commented Jun 1, 2021

My regex should have had to filter them, anyway I restore those lines back.
The CodePolicy check fails but in my local environment it succeed, do you have any hint?

@bschmalhofer
Copy link
Contributor Author

Hi Nicola,

I have updated the instructions for running the test. https://github.com/bschmalhofer/otobo-ideas/wiki/Running-the-Test-Suite should now be more complete.

I have also run the test suite from your PR and got three scripts with failures.

/opt/otobo/scripts/test/EmailParser.t                                                                     (Wstat: 1024 Tests: 125 Failed: 4)
  Failed tests:  32, 61-62, 95
  Non-zero exit status: 4
/opt/otobo/scripts/test/PostMaster.t                                                                      (Wstat: 6144 Tests: 2841 Failed: 24)
  Failed tests:  72, 124, 300, 352, 528, 580, 756, 808, 984
                1036, 1212, 1264, 1440, 1492, 1668, 1720
                1896, 1948, 2124, 2176, 2352, 2404, 2580
                2632
  Non-zero exit status: 24
/opt/otobo/scripts/test/WebUserAgent.t                                                                    (Wstat: 1024 Tests: 121 Failed: 4)
  Failed tests:  76-77, 89, 101
  Non-zero exit status: 4
Files=951, Tests=84240, 6939 wallclock secs (31.19 usr  3.08 sys + 1539.18 cusr 188.14 csys = 1761.59 CPU)
Result: FAIL
ran tests for product OTOBO 10.0.x on host e0d9192181fc .

The failures in EmailParser.t and in PostMaster.t are just MD5 sums that have changed. That should be easy to fix.
WebUserAgent.t depends on a server at otobo.org that has to respond. I propose that you skip these tests and open a new issue for that case. It might take a while until the infrastructure is set up.

Best regards,
Bernhard

@niccord
Copy link
Contributor

niccord commented Jun 9, 2021

Hi Bernhard,
I use to run this script for the test suite: bin/otobo.Console.pl Dev::UnitTest::Run [--test testFile].
I run it inside the docker container, of course.

With this script, EmailParser, PostMaster and WebUserAgent give a different result:
The first two passes and the last one returns a different failed test list.

So what's the difference between the two scripts?
Is there any way to start only the tests I want to check?

Thank you 🙂

@svenoe
Copy link
Contributor

svenoe commented Jun 10, 2021

Hi Nicola,

without looking at it, probably the scripts compare different files or checksums or so. This can really only be debugged by looking at what the script does, exactly. A good help can be to use --verbose as an additional flag for Dev::UnitTest::Run.

Starting a single test is for example: bin/otobo.Console.pl Dev::UnitTest::Run --test PostMaster/FollowUp (that is the test script without the '.t' and without scripts/test/).

Thank you - Sven

@bschmalhofer
Copy link
Contributor Author

Hi Nicola, @niccord,

I would also recommend that you verify that e.g. /opt/otobo/scripts/test/EmailParser.t really holds your changed file. It is not enough that the container otobo_web_1 is running with your locally built image. The images holds the changed files in /opt/otobo_install/otobo_next. The files still have to be copied to the volume at /opt/otobo. See line 82 of scripts/update.sh in the otobo-docker repository.

# The containers are still stopped.
# Copy the OTOBO software from the potentially changed image into the volume mounted at /opt/otobo.
# The required config is taken from the .env file.
docker-compose run --no-deps --rm web copy_otobo_next

@niccord
Copy link
Contributor

niccord commented Jul 13, 2021

Hi Bernhard @bschmalhofer,
sorry to come back so late to this issue.

I followed your tutorial step by step but I have an issue with docker-compose up -d.
It gets an error from your docker image rotheross/otobo-selenium-chrome:

Pulling selenium-chrome (rotheross/otobo-selenium-chrome:latest)...
ERROR: manifest for rotheross/otobo-selenium-chrome:latest not found: manifest unknown: manifest unknown

Can you please help me?
Thanks.

@bschmalhofer
Copy link
Contributor Author

bschmalhofer commented Jul 13, 2021

Hi Nicola,

sorry, that error comes from recent changes for #1109. The aim there is to simplify the whole process by eliminating scripts/devel/prepare_selenium_container.sh. For that I started on a custom Selenium image, that unfortunately in not yet available on Docker Hub.Updated instructions are now at https://github.com/bschmalhofer/otobo-ideas/wiki/Running-the-Test-Suite-under-Docker.
As a workaround you can specify the locally built image, which is created by running bin/docker/build_docker_images.sh. For that you can add the following to your .env file:

 ################################################################################
 # The Docker image for otobo_selenium_1 can be specified explicitly.
 # The default image is rotheross/otobo-selenium-chrome:latest
 ################################################################################
 
 # For use with scripts/update.sh, otovar_XXX() will be replaced
 OTOBO_IMAGE_OTOBO_SELENIUM_CHROME=otobo-selenium-chrome:local-10.0.x
 

And thanks for you continued contributions to OTOBO.

@niccord
Copy link
Contributor

niccord commented Jul 13, 2021

Thanks for your quick answer.
How can I generate an otobo-selenium-chrome image? I didn't see any in my docker image ls output and I can't find it anywhere.

@bschmalhofer
Copy link
Contributor Author

On which branch are you working? The head of https://github.com/RotherOSS/otobo/blob/rel-10_0 contains the script bin/docker/build_docker_images.sh that should build this local image. Maybe you need to rebase your local branch.

@niccord
Copy link
Contributor

niccord commented Jul 13, 2021

I am working on a branch on my personal fork trying to fix this issue.
I am going to try rebasing, thanks.

@bschmalhofer bschmalhofer self-assigned this Aug 27, 2021
@bschmalhofer
Copy link
Contributor Author

Picked this up again after a pause and plan to have it ready for 10.0.13. As far as I rember some MD5 hashes still need to be adapted in the test suite.

@niccord
Copy link
Contributor

niccord commented Aug 27, 2021

Hi Bernhard,
I should have fixed those MD5 failing tests.

@bschmalhofer
Copy link
Contributor Author

Merged Nicolas PR, with the fixed MD5 sums. Reopening because I switched some more references from OTRS to OTOBO.

@bschmalhofer bschmalhofer reopened this Aug 28, 2021
bschmalhofer added a commit that referenced this issue Aug 28, 2021
at the "Gründerzentrum im Hafen Straubing-Sand"
bschmalhofer added a commit that referenced this issue Aug 28, 2021
Moving from Straubig-Sand to Oberwalting.
bschmalhofer added a commit that referenced this issue Aug 28, 2021
@bschmalhofer
Copy link
Contributor Author

Eliminiated some more references to OTRS. Closing this issue.

@svenoe
Copy link
Contributor

svenoe commented Nov 3, 2021

Wrong commit messages...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers tidying Tidying of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants