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

Reduce AZP build to a minimal subset of core, except when running on Windows #2587

Merged
merged 12 commits into from Apr 19, 2020

Conversation

rnorth
Copy link
Member

@rnorth rnorth commented Apr 18, 2020

No description provided.

@rnorth rnorth changed the title Reduce AZP build to a minimal subset of core, except when running on … Reduce AZP build to a minimal subset of core, except when running on Windows Apr 18, 2020

# Run all tests when running the Windows CI tests
- job: full_build_windows
condition: eq(variables['Agent.OS'], 'Windows')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is right, but obviously want to test it.

@rnorth rnorth added this to the next milestone Apr 18, 2020
@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

AZP build failed - will investigate in a little while.

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

/azp run Windows 10 - Docker for Windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

/azp run Windows 10 - Docker for Windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

@bsideup I'm not certain, but I think we might need a variable to be set on the Windows build agent.

This conditional:

# Run all tests when running the Windows CI tests
- job: full_build_windows
  condition: eq(variables['Agent.OS'], 'Windows_NT')

seems to be evaluating to:

Expanded: eq(Null, 'Windows_NT')
Result: False

Agent.OS is apparently a standard property, but I wonder if that's only true for Azure-provided build workers.

Any thoughts?

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

/azp run Windows 10 - Docker for Windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

/azp run Windows 10 - Docker for Windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

I think I have a solution to the problem now - Agent.OS is only available within a job. So I've contracted the two different Linux/Windows builds into a single job, with a conditional on each step to decide which scale of build to run.

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

/azp run Windows 10 - Docker for Windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

options: '--no-daemon --continue'
tasks: 'testcontainers:check'
options: "--no-daemon --continue"
tasks: "testcontainers:test --tests GenericContainerRuleTest"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we can change it based on conditions (all on Windows, subset otherwise)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so too, and have been scouring the documentation to try and find out how to do this, but so far no luck. Have you seen anything concrete?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, I'd missed that - cheers.

Copy link
Member Author

@rnorth rnorth Apr 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Agent.OS variable is not set at the point in time when conditional insertion takes place, so I was unable to set a variable.

For example:

variables:
  ${{ if eq(variables['Agent.OS'], 'Linux') }}:
    # Run minimal core subset of tests on Linux
    tasks: "testcontainers:test --tests GenericContainerRuleTest"
  ${{ if eq(variables['Agent.OS'], 'Windows_NT') }}:
    # Run all checks on Windows
    tasks: "check"

would effectively do nothing, because neither branch is actually true. The tasks variable was unset at the time of use in the Gradle@2 task, and caused an error when trying to use it.

Just in case something else was the problem, I tried:

variables:
  ${{ if eq(variables['Agent.OS'], 'Linux') }}:
    # Run minimal core subset of tests on Linux
    tasks: "testcontainers:test --tests GenericContainerRuleTest"
  ${{ if ne(variables['Agent.OS'], 'Windows_NT') }}:
    # Run all checks on Windows
    tasks: "check"

(the difference is ne, not eq, on the second branch).

With this, the tasks variable was set, but always to "check". So this demonstrates that the conditional insertion is working, and the placeholder resolution is working - the problem is just that Agent.OS is not set when the pipeline file is compiled, which is before it hits a build agent.

I rolled back the branch to get rid of the messy trail of commits that I created while trying to figure this out! 😅

@rnorth
Copy link
Member Author

rnorth commented Apr 18, 2020

/azp run Windows 10 - Docker for Windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rnorth rnorth merged commit c969200 into master Apr 19, 2020
@rnorth rnorth deleted the reduce-azp-build branch April 19, 2020 14:22
quincy pushed a commit to quincy/testcontainers-java that referenced this pull request May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants