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

Issue 4896 - improve CI tests report in case of SERVER_DOWN exception #4897

Merged
merged 15 commits into from
Oct 1, 2021

Conversation

progier389
Copy link
Contributor

** Description **
These first change implements an hook that add some data in the report if a test fails with ldap.SERVER_DOWN exception
(Typically a SERVER_DOWN section is added that contains ns-slapd core files digest and extract of important message from error log)
This also change the default to mdb so the PR could be used to run the CI - Note: should revert that change before the final merge )

@progier389
Copy link
Contributor Author

In the first few test I looked at, the SERVER_DOWN was coming after a MDB_MAP_FULL error The 1GB default limit is probably too small
In some other test the test just is not adapted to mdb ( for example checking the justReplication flag while compacting the db - single mdb map means that we have to compact all or nohing ==> modifying the test to keep some of the assertions in bdb mode only )
Strange error in automember (result moved from UNWILLING TO PERFORM to OPERATIONS_ERROR which seems weird)

@Firstyear
Copy link
Contributor

Can you provide an example of what this output will look like in a failure case? I think it's a really good idea though.

@progier389
Copy link
Contributor Author

Hi William,
you have an example in basic test result:
For example https://github.com/progier389/389-ds-base/runs/3525756996?check_suite_focus=true

Note: I think I will change the way to discover the topologies (looking up for all existing dse.ldif seems a better idea) because current code does not provide any feedback on failed setup

@Firstyear
Copy link
Contributor

Hi William,
you have an example in basic test result:
For example https://github.com/progier389/389-ds-base/runs/3525756996?check_suite_focus=true

That's actually really impressive and looks great. If there is a crash it may also be worth listing the asan log dir?

Note: I think I will change the way to discover the topologies (looking up for all existing dse.ldif seems a better idea) because current code does not provide any feedback on failed setup

A failed setup is a bit of a tricky case, but couldn't that be covered in the topology setup code to capture when an exception happens and then trigger the same routines to display the info?

@progier389
Copy link
Contributor Author

I agree, asan results should be logged too .

A failed setup is a bit of a tricky case, but couldn't that be covered in the topology setup code to capture when an exception happens and then trigger the same routines to display the info?

In fact according to my test, the current code already capture the setup exception,
the issue is that the topology is not registered in the __topologies list (as create_topology never returns ... ) so there are no instances to check.

That is why I think that I should walk all dse files and generate the associated offline DirServ rather than registering the created topologies in a list

@vashirov
Copy link
Member

vashirov commented Sep 8, 2021

I agree, asan results should be logged too .

I dusted off and rebased my PR #4625 that adds ASAN pipeline and logs results as an artifact.

@Firstyear
Copy link
Contributor

In that case ack from me @progier389 :)

@vashirov I'll review that seperately :)

@progier389 progier389 changed the title Issue 4896 - CI tests on lmdb still reports errors Issue 4896 - improve CI tests report in case of SERVER_DOWN exception Sep 15, 2021
@progier389
Copy link
Contributor Author

Added asan sections and dbscan -L output (which IMHO could be useful when getting out of disk or db map size because the number of records per db instance is listed)
Also changed the way to list the instances (getting local instance list rather than using topology)
And finally since the Topology dependency is removed I moved the Report generation code in a report.py file near conftest.py
(I think that splitting the code is more readable and IMHO lib389 is not the right place for pytest specific code )

@progier389 progier389 merged commit 6cac256 into 389ds:master Oct 1, 2021
@progier389 progier389 deleted the i4896 branch July 27, 2022 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants