Make Gitlab::Ci::Config::Entry::Node#inspect human readable

For now Gitlab::Ci::Config::Entry::Node#inspect is not readable therefore making debugging harder. In gitlab-ce!9666 a simple version is implemented as follow:

def inspect
  val = leaf? ? config : descendants
  unspecified = specified? ? '' : '(unspecified) '
  "#<#{self.class.name} #{unspecified}{#{key}: #{val.inspect}}>"
end

This would show something like:

#<Job {job0: [#<Stage {stage: "test"}>]}>

However this is misleading, because it should actually show:

#<Job {job0: [#<Stage (unspecified) {stage: "test"}>]}>

The reason why it's always showing specified instead of properly showing unspecified, is because Gitlab::Ci::Config::Entry::Unspecified is a wrapper around Stage. Therefore whenever Unspecified#inspect is called, it's delegating to Stage#inspect, effectively negate the effect of the wrapper.

Originally, I solved this by removing Unspecified, and added an attribute to the Node to indicate it's specified or not. See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9666#note_24608178

However we don't yet have consensus about what we should do here, and it's unrelated to that particular merge request therefore I just reverted it.

Here's what I think we could do:

  • Remove Unspecified and add an attribute to Node as I originally did. This was chosen not because I think this was the best approach, just because that's what came to my mind first and I wanted to complete the merge request sooner than later. This is no longer a concern. The downside of doing so is that then we're manipulating states, which is bad.
  • Also implement inspect for Unspecified. This way specified? could be properly detected. The downside of doing so is that we'll need to implement inspect twice, identically. We could reduce repetition by using a module and include (or prepend) it, but this doesn't feel right to me either.

Can we find a better solution here?

/cc @grzesiek

Edited Aug 30, 2025 by 🤖 GitLab Bot 🤖
Assignee Loading
Time tracking Loading