[Bro-Commits] [git/bro] master: Updating some test baselines. (827dcea)

   - I see 5 broken tests currently.

Me, too:

doc.coverage … failed

This will fail whenever new bro scripts are added, but it's easy to fix: as the test comments indicate, run the `doc/scripts/genDocSourcesList.sh` script. Maybe later, if this is reliable enough, we can make it happen automatically on commits or something.

scripts.bare-mode-coverage … failed

This is an indicator that there's some errors when loading individual scripts in bare-mode. It's not going to fully work until something is done about http://tracker.bro-ids.org/bro/ticket/544 (hot.conn.bro & scan.bro), but the diagnostic output is still useful to correct other errors.

scripts.test-all-policy-coverage … failed
scripts.base.init-default-coverage … failed

Also get out of date when new scripts are added to either policy/ or base/ -- fixing them should be inside your domain? :slight_smile:

scripts.base.protocols.http.http-header-crlf … failed

I have a fix waiting merge in #488.

So we're not far away from having everything pass.

- Jon

Yippie. :slight_smile:

Let's put the notes re the tests that need updating when scripts
change into something like "btest/NOTES.tests". That's a good
reference on what to do.

Robin

re the tests that need updating when scripts
change into something like "btest/NOTES.tests". That's a good
reference on what to do.

I think it's better if the test file itself has some comments about how to correct failures that might be fairly regular because it's the more obvious place to look for someone trying to fix a failure (they're going to look at what the test actually does and find the comments that way), plus if someone changes the test, they're more likely to remember to update the documentation that goes along with it if it's in the same file.

- Jon

scripts.bare-mode-coverage … failed

This is an indicator that there's some errors when loading individual scripts in bare-mode. It's not going to fully work until something is done about http://tracker.bro-ids.org/bro/ticket/544 (hot.conn.bro & scan.bro),

I'm working on those.

So we're not far away from having everything pass.

Thanks for the notes and I agree with Robin that having those documented in a readme would be awesome. I suppose it should also be added to the developer documentation.

  .Seth

I think both would be good. The NOTES files is more helpful to change
things in advance, i.e., "I just added a script; which tests do I need
to adapt?". It would be fine though to just list the relevant tests
there, and leave the docs on what specficially to do in the test files
itself so that we don't have two places to keep in sync.

Robin

I think both would be good. The NOTES files is more helpful to change
things in advance, i.e., "I just added a script; which tests do I need
to adapt?".

Changing things in advance without actually running the tests to make sure it works as expected seems dangerous. I was thinking that part of the standard development process would be for one to run the test suite themselves before requesting a merge (or doing a merge) to master. And in that case, the question is answered by the act of just running the test suite.

If running the test suite manually isn't part of the process, then I don't think any extra amount of documentation is going to help prevent new script commits resulting in breaking of tests (the automated NMI B&T is best used to catch platform-specific problems, not as a general way to catch problems after they've already been committed.)

- Jon

That's not what I wanted to say. :slight_smile: Of course one still needs to run
the tests before pushing things further upstream. But it's easier to
first adapt the tests and then run them, as otherwise one gets the
failures and then needs to decide one by one whether it's expected to
break or a new problem. Better to get the expected stuff out of the
way first.

Robin

Changing things in advance without actually running the tests to make
sure it works as expected seems dangerous.

That's not what I wanted to say. :slight_smile: Of course one still needs to run
the tests before pushing things further upstream.

Ah, ok.

But it's easier to first adapt the tests and then run them

Maybe it's just more my style, but I'd rather see what the failure looks like first before adapting anything. Otherwise I might be adapting something that didn't need adapting or adapting it in the wrong way such that I'll still have to fix it later.

I'm still thinking that a NOTES file that maintains a list of tests that are expected to fail more often because they're coverage-type tests is going become out-of-date easily. Supporting evidence: I just noticed that the testing/btest/README is in need of significant updating because it tried to maintain a list of major test directories.

Right now all the tests that are expected or more likely to break when new code is added have "coverage" somewhere in the name. What do you think about grouping them all in a single "coverage" directory to keep that distinction, but then also provide a canonical place for preemptive test-fixers to look?

- Jon

Ok, I like that.

Robun