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

Make test work locally #90

Merged
merged 7 commits into from
Jul 21, 2021

Conversation

cvvergara
Copy link
Contributor

@cvvergara cvvergara commented Jul 21, 2021

This is the first PR of a series of PR
Before any change, I should be able to run tests locally on my computer.
The main issue for not been able to test locally is that pg_ctl (int test.sh) is not found, unless you add it to the manually to the path, now, that file is configured with the findings during the build and configured.

The tests are grouped now in directory test, on lack of a better name I am calling "general" the items that are not point or npoint

test
├── general
│   ├── data
│   ├── expected
│   └── queries
├── npoint
│   ├── data
│   ├── expected
│   └── queries
├── point
│   ├── data
│   ├── expected
│   └── queries
└── scripts

I can not how things went travis
https://travis-ci.com/github/cvvergara/MobilityDB

@cvvergara cvvergara requested review from estebanzimanyi, mschoema and alesuiss and removed request for mschoema July 21, 2021 00:56
@cvvergara cvvergara added enhancement New feature or request Build labels Jul 21, 2021
@estebanzimanyi
Copy link
Member

Many thanks for this amazing work !

I have one question. A traditional usage when developing a new functionality in MobilityDB is to test a single file, or a set of files satisfying a regular expression. For example, while the command
ctest
launch all the test, the command
ctest -R 56
will only launch the tests 56_tpoint_spatialfuncs and 56_tpoint_spatialfuncs_tbl.

Is this functionality available in your PR ?

@estebanzimanyi
Copy link
Member

estebanzimanyi commented Jul 21, 2021

In the file .github/workflows/pgversion.yml I would suggest to change the test database from ___pgr___test___ to ___mobdb___test___

@estebanzimanyi
Copy link
Member

  • In file test/scripts/CMakeLists.txt
#TODO find xycat (or suitable decompressor)
set(UNCOMPRESS xycat)

I think xycat should be xzcat

  • In files point/point.cmake and npoint/npoint.cmake we can remove

add_definitions(-DWITH_POSTGIS)

since this is no longer needed in the code.

  • In file test/scripts/CMakeLists.txt, line 33, I think there is a typo: buid -> build

  • In file test/scripts/test.sh we can remove the commented line 79, it was needed before fixing the name of the extension to mobilitydb

@cvvergara
Copy link
Contributor Author

About:

  • In files point/point.cmake and npoint/npoint.cmake we can remove add_definitions(-DWITH_POSTGIS)

    • At least I am not doing it at this stage, as I haven't check all the build. (and those files eventually will be 100% gone)
  • In file test/scripts/test.sh we can remove the commented line 79, it was needed before fixing the name of the extension to mobilitydb

    • I still need it to test when the \echo Use "CREATE EXTENSION mobilitydb" to load this file. \quit line is added to make sure that the extension does not get loaded without create extension

@cvvergara
Copy link
Contributor Author

Please let me do my own merges.
Just approve, and I'll do the merge

@estebanzimanyi
Copy link
Member

Arthur Lesuisse, our previous sysadmin, works on another department of the university. Therefore he maybe will not have time to do this review. If needed, a second review can be done by Maxime Schoemans.

@cvvergara cvvergara merged commit 759a2ee into MobilityDB:develop Jul 21, 2021
@cvvergara cvvergara deleted the make-test-work-locally branch July 21, 2021 15:45
@cvvergara
Copy link
Contributor Author

In pgRouting I list many people, but when I see that it is approved by anyone, I do the merge

@cvvergara cvvergara mentioned this pull request Jul 22, 2021
10 tasks
@estebanzimanyi estebanzimanyi mentioned this pull request Jul 25, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants