Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

[FIX] test_server.py: Adding more validation to detected ERROR #476

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JesusZapata
Copy link
Member

@JesusZapata JesusZapata commented Aug 8, 2017

Relating to #447 (comment)

With this small change now travis is red if in the log has one ERROR

I made one PR of test related with this JesusZapata/project#1

image

And now the travis is red

Fix #447

@JesusZapata
Copy link
Member Author

@dreispt @moylop260 @sbidoul
Yours comments are welcome

@dreispt
Copy link
Member

dreispt commented Aug 8, 2017

Sometimes we test if an error is raised.
Will this incorrectly break such a build?

@JesusZapata JesusZapata changed the title [FIX] test_server.py: Adding more validation to detected ERROR and WARNING [FIX] test_server.py: Adding more validation to detected ERROR Aug 8, 2017
@JesusZapata
Copy link
Member Author

@dreispt
The execution of odoo continues running without paying attention to the error
I don't know if this behaviour is the correct

@sbidoul
Copy link
Member

sbidoul commented Aug 9, 2017

I also noticed that, but I never dared to change it, as I supposed that if someone explicitly filtered on CRITICAL errors only, there must have been a reason.

FWIW, for our own CI needs, we created a script dedicated to checking odoo logs, with agressive defaults (WARNING level records are considered errors) and a regex-based mechanism to filter expected errors.

@dreispt
Copy link
Member

dreispt commented Aug 10, 2017

To be clear, I'm not against this change as long as long as tests verifying if a Exception was raised don't incorrectly break the build.
Can you check that @JesusZapata ?

@nhomar
Copy link
Member

nhomar commented Oct 30, 2017

FWIW, for our own CI needs, we created a script dedicated to checking odoo logs, with agressive defaults (WARNING level records are considered errors) and a regex-based mechanism to filter expected errors.

@sbidoul that's more or less what we do internally hacking a little mqt itself.

@moylop260 is this PR valid to be merged as it is? I am worried of change the level and bring false green/red on this.

I prefer move this to a future version, What do you think?

@nhomar nhomar mentioned this pull request Oct 30, 2017
@lasley
Copy link
Contributor

lasley commented Jan 3, 2018

Hi @JesusZapata & @moylop260 - any update on this?

@lasley lasley added the question label Jan 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants