fix SLURM env setup

[johrstrom]

fix SLURM environment setup such that --export=NONE is the default
(as it was before with SBATCH_EXPORT=NONE), job_environment adds
correctly to --export, and copy_environment correctly prefixes with ALL.

comment also left in the code as to why we default to NONE.

Fixes the issues raised in and created by #109 and #158.

[ericfranz]

By fixing it so we default to export NONE, how will that impact sites that already observed this problem and "fixed it". Will their "fixes" still work, or will they negatively affected by the switch again?

[ericfranz]

Is there a better name than base_args? Just by going by the name of the method, it isn't clear why every test wraps the args to test against with base_args except for the test context "with :job_environment" do where it is omitted and args: ["--export", "key"] is used. It looks like the fact that it isn't used in the latter test is a bug but of course adding it would break the test.

I don't know the answer besides a larger refactor that would change/simplify how we test these things.

[ericfranz]

It would be better to get rid of base_args and inline with the rest of the tests. Since we arbitrarily add base args to the tests, it isn't the right abstraction. Better to have duplication than the wrong abstraction. A better architecture for the adapter code in the future will address the problems with the tests.

So

    context "with :queue_name" do
      before { adapter.submit(build_script(queue_name: "queue")) }

-     it { expect(slurm).to have_received(:submit_string).with(content, args: ["-p", "queue"], env: {}) }
+     it { expect(slurm).to have_received(:submit_string).with(content, args: ["-p", "queue", "--export", "NONE"], env: {}) }
    end

[johrstrom]

I can test it to be doubly sure, but the 'fix' was to source files under /etc/profile.d. Given that --export=NONE seems to already do this, sourcing the file(s) a second time would not seem to change anything.

Which is to say, sites with the 'fix' wouldn't be impacted other than having a little bit of tech debt to clean up. But I'll confirm.

[ericfranz]

Besides getting rid of the base_args in the test, I'm okay with this merging when you are satisfied that the sites that have addressed the temporary behavior change will not be negatively affected.

[johrstrom]

I've been double checking this morning and I think we're OK with the 2nd sourcing of files. The site I was working on even had it in an if block (if -z $LMOD_VERSION), but again, sourcing a profile file a second time shouldn't cause any problem.