break the build if Undeclared detected about smalltalkci HOT 18 CLOSED

Interesting idea. I guess we could add support for something like #failOnWarnings to SCIMetacelloLoadSpec and then you could enable it by setting the value to True.

I wonder what @dalehenrich thinks about this.

BTW: I literally just made smalltalkCI ignore warnings (0a1f772) 😁 .

from smalltalkci.

My thoughts are that we'd like to fail on specific warnings rather than generic warnings ... @DamienCassou could you supply a stack that shows an example of the Warning you want ignored? If this is coming from Metacello, I'd be willing to create a specific class so that we can selectively fail ...

from smalltalkci.

I have the impression the warning comes from the compiler, in particular the Encoder>>undeclared: which signals an UndeclaredVariableWarning. This exception can be resumed. In Pharo, when Monticello loads a package containing a method with a reference to a non existing class, a message is shown in the transcript and loading continues. In my example, I have a package Foo-Main with a class FooMain with a method foo referencing a class in another package. If this dependency is not loaded, loading Foo-Main results in the following being shown in the transcript:

FooMain>>foo (FooDependency is Undeclared) 

from smalltalkci.

Metacello, in MetacelloMCVersionValidator>>validateCleanLoadAndTestsForVersion:loads: has code related to this exception as well.

from smalltalkci.

As I think about the UndeclaredVariableWarning, I'm pretty sure that the Warning can be thrown in cases where the global will be resolved by another package being loaded a bit later during the same project load ... so it is not fool proof ...

I deal with this in GemStone by having a test that detects sent but no implemented methods and undeclared globals ...

from smalltalkci.

Marcus told me the class Encoder is not used anymore, it's the old compiler. What is important now is that the new compiler signals OCUndeclaredVariableWarning when a method references an undeclared variable. I would like the code loader to signal an error if this warning is signaled. Is that possible?

from smalltalkci.

@dalehenrich you are right. But this warning typically indicates a bad dependency: if my packages and configuration are correct, there should not be any undeclared variable at any point of the loading process; Metacello should load packages in the right order. If there is an undeclared variable at any point, it means the configuration is incorrect (a dependency is not yet loaded) or there are circular dependencies. I would like the build to break so that I know I have a problem somewhere.

from smalltalkci.

@DamienCassou you are correct that it is a "bad dependency", but as you are aware, the universe is full of projects with bad dependencies that end up loading correctly...

Given that you have a specific exception to be caught, perhaps it makes sense to allow one to specify a list of Notifications that will cause a build to fail:

SmalltalkCISpec {
  #loading : [
    SCIMetacelloLoadSpec {
      #failLoadOn : [ 'OCUndeclaredVariableWarning' ],
      #baseline : 'MyProject',
      #directory : 'packages',
      #platforms : [ #squeak, #pharo, #gemstone ]
    }
  ]
}

from smalltalkci.

@DamienCassou you are correct that it is a "bad dependency", but as you are aware, the universe is full of projects with bad dependencies that end up loading correctly...

I agree, that's why I'm not proposing to fail all builds because of that. But I want my packages to be free of such problems. i think your proposal makes sense.

from smalltalkci.

@fniephaus what do you think of #failLoadOn ... I could take this on if you'd like ... it's related to the #onWarningLog option ...

from smalltalkci.

Please do! This is an open source project and everyone is invited to contribute ;)

from smalltalkci.

👍 for having this option

from smalltalkci.

Does Metacello already have something similar to #failLoadOn or would you just catch the user-provided warnings manually and fail the build?

from smalltalkci.

No ... I was thinking that SmalltalkCI would need to handle that ...

from smalltalkci.

Thanks for the info, so we'd need to do this manually in SCIMetacelloLoadSpec>>basicLoadProjectOn:.

from smalltalkci.

@dalehenrich what do you think about this?

from smalltalkci.

@fniephaus I think I would like a stack or at least an error message stating why the build failed.

from smalltalkci.

Ups, good point...

from smalltalkci.